From 61327fedbe841ded3d2b58ca44e14beb8625841b Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 16:44:09 -0400 Subject: [PATCH 01/12] Invoke the print command in the builtin-name codegen test The print_command case passed --version and never ran the generated command. It now runs the command and asserts its output; --version stays as its own case. Co-Authored-By: Claude Opus 5.5 --- tests/test_codegen.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_codegen.py b/tests/test_codegen.py index 53a2323..ef621d0 100644 --- a/tests/test_codegen.py +++ b/tests/test_codegen.py @@ -214,7 +214,11 @@ def test_manifest_text_is_not_executed(run, monkeypatch, manifest): "['a', 'b']", id="builtin_command", ), - pytest.param(BASE + "commands:\n print: print('printed')\n", ["--version"], "t, 1", id="print_command"), + pytest.param(BASE + "commands:\n print: print('printed')\n", ["print"], "printed", id="print_command"), + # --version is implemented with print(): a command named `print` must not replace it + pytest.param( + BASE + "commands:\n print: print('printed')\n", ["--version"], "t, 1", id="print_command_version" + ), pytest.param(BASE + "commands:\n my-group.sub: print(1)\n", ["my-group", "sub"], "1", id="dash_group"), pytest.param(BASE + "commands:\n 2fa.setup: print(1)\n", ["2fa", "setup"], "1", id="digit_group"), pytest.param( From 1459ad266cbcb28806f3d9165ffd1f813bfa4519 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 16:48:47 -0400 Subject: [PATCH 02/12] Keep loaded-CLI names, launchers and removal inside cliffy's directories - A name given to rm, update, info, build and docs is validated before it is used in a path: ../x no longer selects metadata, modules or launchers outside of cliffy's directories. homer and Loader refuse such names themselves. - A launcher is only cliffy's when it is a regular file, not a link, whose text is the launcher of exactly that CLI. A file that merely contains 'from cliffy.clis.' is no longer overwritten or deleted. - Launchers and modules are written to a temporary file and renamed into place. A new launcher is created exclusively; links are never followed. - cli rm removes the launcher, the module and then the metadata record. When the launcher is not cliffy's, nothing is removed and the CLI stays listed and runnable. cli rm-all exits with 1 when a CLI could not be removed. - The symlink test skips where symbolic links cannot be created. Co-Authored-By: Claude Opus 5.5 --- cliffy/cli.py | 50 ++++++++--- cliffy/helper.py | 63 ++++++++++++++ cliffy/homer.py | 17 +++- cliffy/loader.py | 120 +++++++++++++++++++++----- tests/test_helper.py | 43 ++++++++++ tests/test_homer.py | 26 ++++++ tests/test_loader.py | 199 ++++++++++++++++++++++++++++++++++++++++++- 7 files changed, 483 insertions(+), 35 deletions(-) diff --git a/cliffy/cli.py b/cliffy/cli.py index d6dff7a..2584cf6 100644 --- a/cliffy/cli.py +++ b/cliffy/cli.py @@ -23,6 +23,7 @@ CLI_NAME_PATTERN, ManifestOrCLI, age_datetime, + cli_name_problem, cliffy_version, exit_err, indent_block, @@ -31,7 +32,7 @@ write_to_file, ) from cliffy.homer import get_clis, get_metadata, get_metadata_path, remove_metadata, save_metadata -from cliffy.loader import Loader +from cliffy.loader import Loader, LoaderError from cliffy.manifest import CLIManifest from cliffy.reloader import Reloader from cliffy.rich import Console, click, print_rich_table # type: ignore @@ -94,7 +95,10 @@ def update(cli_names: list[str]) -> None: """Reloads CLI by name""" failed = False for cli_name in cli_names: - if cli_metadata := get_metadata(cli_name): + if problem := cli_name_problem(cli_name): + out_err(f"~ {problem}") + failed = True + elif cli_metadata := get_metadata(cli_name): with open(cli_metadata.runner_path, encoding="utf-8") as manifest: T = Transformer(manifest, blueprint=cli_metadata.blueprint) Loader.load_from_cli(T.cli) @@ -193,24 +197,40 @@ def remove(cli_names: list[str]) -> None: """Remove a loaded CLI by name""" failed = False for cli_name in cli_names: - if get_metadata_path(cli_name): - remove_metadata(cli_name) - Loader.unload_cli(cli_name) - out(f"~ {cli_name} removed 💥", fg="green") - else: + if problem := cli_name_problem(cli_name): + out_err(f"~ {problem}") + failed = True + elif not get_metadata_path(cli_name): out_err(f"~ {cli_name} not loaded") failed = True + elif not remove_loaded_cli(cli_name): + failed = True if failed: raise SystemExit(1) +def remove_loaded_cli(cli_name: str) -> bool: + """Remove the launcher, the module and the metadata record of a CLI. All of it, or none of it. + + The record goes last: as long as something of the CLI is left, it is listed and can be removed again. + """ + try: + Loader.unload_cli(cli_name) + except (LoaderError, ValueError) as e: + out_err(f"~ {cli_name} not removed: {e}") + return False + remove_metadata(cli_name) + out(f"~ {cli_name} removed 💥", fg="green") + return True + + def remove_all() -> None: """Remove all loaded CLIs""" - for metadata in get_clis(): - remove_metadata(metadata.cli_name) - Loader.unload_cli(metadata.cli_name) - out(f"~ {metadata.cli_name} removed 💥", fg="green") + # all of them are tried. The command fails when one could not be removed + removed = [remove_loaded_cli(metadata.cli_name) for metadata in list(get_clis())] + if not all(removed): + raise SystemExit(1) @click.argument("cli_or_manifests", type=ManifestOrCLI(), nargs=-1) @@ -252,6 +272,10 @@ def build( cli_name = cli.name else: cli_name = cli_or_manifest + if problem := cli_name_problem(cli_name): + out_err(f"~ {problem}. A manifest is given as a path that ends in `yaml`") + failed = True + continue if not (metadata := get_metadata(cli_name)): out_err(f"~ {cli_name} not loaded") failed = True @@ -288,6 +312,8 @@ def build( @click.argument("cli_name", type=str) def info(cli_name: str) -> None: """Display CLI info""" + if problem := cli_name_problem(cli_name): + exit_err(f"~ {problem}") metadata = get_metadata(cli_name) or exit_err(f"~ {cli_name} not loaded") out(f"{click.style('name:', fg='blue')} {metadata.cli_name}") out(f"{click.style('version:', fg='blue')} {metadata.version}") @@ -422,6 +448,8 @@ def docs(cli_or_manifest: str, format: str, output_dir: str, blueprint: str | No doc_generator.generate(format, output_dir) out(f"+ {T.cli.name}.{format}") else: + if problem := cli_name_problem(cli_or_manifest): + exit_err(f"~ {problem}. A manifest is given as a path that ends in `yaml`") metadata = get_metadata(cli_or_manifest) or exit_err(f"~ {cli_or_manifest} not loaded") # metadata.manifest is the raw manifest text. It still has to be rendered and merged with its # includes, so it is transformed from its source like `cli update` does. diff --git a/cliffy/helper.py b/cliffy/helper.py index b69a98e..aa4e056 100644 --- a/cliffy/helper.py +++ b/cliffy/helper.py @@ -5,6 +5,7 @@ import os import platform import re +import secrets import sys from datetime import datetime from importlib.metadata import PackageNotFoundError, distributions, version @@ -65,6 +66,27 @@ def convert( # type: ignore[override] return value +def cli_name_problem(name: object) -> str | None: + """Why `name` cannot be the name of a CLI, or None when it can. + + A name becomes a file name in three directories (launcher, module, metadata record). Whatever a + command is given as the name of a loaded CLI goes through this first, so that `..` or a path + separator cannot make it point at a file outside of them. + """ + if isinstance(name, str) and CLI_NAME_PATTERN.fullmatch(name): + return None + return ( + f"{name!r} is not a valid CLI name: a name has letters, digits, `_` and `-` only " + "and starts with a letter or `_`" + ) + + +def validate_cli_name(name: str) -> str: + if problem := cli_name_problem(name): + raise ValueError(problem) + return name + + def write_to_file(path: str, text: str, executable: bool = False) -> None: output_file = Path(path) output_file.parent.mkdir(exist_ok=True, parents=True) @@ -75,6 +97,47 @@ def write_to_file(path: str, text: str, executable: bool = False) -> None: make_executable(path) +def write_atomically(path: str, text: str, executable: bool = False, replace: bool = True) -> None: + """Write a file so that `path` is never seen half-written and a symbolic link at `path` is never followed. + + The text goes to a new file in the same directory, which is then moved onto `path`: whatever is + at `path`, a link included, is replaced as a directory entry, not written through. + + Args: + replace: With False the file is only created. FileExistsError is raised when something is at + `path` already, also when it appeared a moment ago, and that file is left alone. + """ + directory = os.path.dirname(path) or "." + os.makedirs(directory, exist_ok=True) + temp_path = os.path.join(directory, f".{os.path.basename(path)}.{os.getpid()}.{secrets.token_hex(4)}.tmp") + no_follow = getattr(os, "O_NOFOLLOW", 0) + # the mode is reduced by the umask, like that of any new file + mode = 0o777 if executable else 0o666 + + def create(target: str) -> None: + descriptor = os.open(target, os.O_WRONLY | os.O_CREAT | os.O_EXCL | no_follow, mode) + # generated code and manifests are always UTF-8, whatever the locale encoding is + with os.fdopen(descriptor, "w", encoding="utf-8") as file: + file.write(text) + + try: + create(temp_path) + if replace: + os.replace(temp_path, path) + return + try: + # a hard link is only made when nothing has the name + os.link(temp_path, path) + except FileExistsError: + raise + except (OSError, NotImplementedError): + # a file system without hard links: created exclusively instead + create(path) + finally: + with contextlib.suppress(FileNotFoundError): + os.unlink(temp_path) + + def import_module_from_path(filepath: str) -> ModuleType: try: path = Path(filepath) diff --git a/cliffy/homer.py b/cliffy/homer.py index e2a5acf..466de2e 100644 --- a/cliffy/homer.py +++ b/cliffy/homer.py @@ -8,7 +8,7 @@ from pathlib import Path from cliffy.commander import CLI -from cliffy.helper import CLIFFY_METADATA_DIR, write_to_file +from cliffy.helper import CLIFFY_METADATA_DIR, cli_name_problem, write_to_file from cliffy.manifest import CLIMetadata @@ -46,6 +46,9 @@ def remove_metadata(cli_name: str) -> None: Args: cli_name (str): CLI name """ + if cli_name_problem(cli_name): + # i.e. `../x`: not the name of a CLI, and it would select files outside of the metadata directory + return # the name is matched literally: `*` or `?` in it must not select other CLIs metadata_paths = glob.glob(f"{glob.escape(str(CLIFFY_METADATA_DIR))}/*/{glob.escape(cli_name)}.*") for path in metadata_paths: @@ -102,6 +105,9 @@ def get_metadata_path(cli_name: str) -> str | None: Returns: Optional[str]: CLI metadata path """ + if cli_name_problem(cli_name): + # i.e. `../x`: not the name of a CLI, and it would select files outside of the metadata directory + return None if cli_metadata_path := glob.glob(f"{glob.escape(str(CLIFFY_METADATA_DIR))}/*/{glob.escape(cli_name)}.json"): return cli_metadata_path[0] return None @@ -115,4 +121,11 @@ def get_clis() -> Iterator[CLIMetadata]: """ metadata_paths = Path(str(CLIFFY_METADATA_DIR)).glob("*/*.json") for metadata_path in metadata_paths: - yield get_metadata_bypath(metadata_path) + metadata = get_metadata_bypath(metadata_path) + # A CLI is known by the name of its record file. The name inside the record is not used to + # find or remove files: an edited record cannot point `cli rm-all` at another CLI. + yield ( + metadata + if metadata.cli_name == metadata_path.stem + else metadata.model_copy(update={"cli_name": metadata_path.stem}) + ) diff --git a/cliffy/loader.py b/cliffy/loader.py index dcebd62..43f61d1 100644 --- a/cliffy/loader.py +++ b/cliffy/loader.py @@ -1,8 +1,24 @@ import contextlib import os +import stat +from typing import Literal from cliffy.commander import CLI -from cliffy.helper import CLIFFY_CLI_DIR, PYTHON_BIN, PYTHON_EXECUTABLE, exit_err, write_to_file +from cliffy.helper import ( + CLIFFY_CLI_DIR, + PYTHON_BIN, + PYTHON_EXECUTABLE, + exit_err, + validate_cli_name, + write_atomically, +) + +ScriptState = Literal["missing", "owned", "foreign"] +NOT_OURS = "already exists and was not created by cliffy, or was changed since" + + +class LoaderError(Exception): + """A loaded CLI could not be removed. The message says what is in the way; nothing was removed.""" class Loader: @@ -13,18 +29,29 @@ def __init__(self, cli: CLI) -> None: def deploy_cli(self) -> str: cli_path = Loader.get_cli_path(self.cli.name) - write_to_file(cli_path, self.cli.code) + write_atomically(cli_path, self.cli.code) return cli_path def deploy_script(self) -> str: + """Write the launcher into the directory of the interpreter's executables. + + A file that is already there is only replaced when it is the launcher cliffy wrote for this + CLI. Anything else stops the load: a manifest named `python` or `pip` would replace that executable. + """ script_path = Loader.get_cli_script_path(self.cli.name) - if os.path.lexists(script_path) and not Loader.is_cli_script(script_path): - # i.e. a manifest named `python` or `pip`: the script would replace that executable - exit_err( - f"~ cannot load {self.cli.name}: {script_path} already exists and was not created by cliffy. " - "Rename the CLI in the manifest" + refused = f"~ cannot load {self.cli.name}: {script_path} {NOT_OURS}. Rename the CLI in the manifest" + state = Loader.script_state(script_path, self.cli.name) + if state == "foreign": + exit_err(refused) + + try: + # A new launcher is created exclusively: a file that appeared since the check is kept. + # An own launcher is replaced by a rename, which never writes through a link. + write_atomically( + script_path, Loader.get_cli_script(self.cli.name), executable=True, replace=state == "owned" ) - write_to_file(script_path, Loader.get_cli_script(self.cli.name), executable=True) + except FileExistsError: + exit_err(refused) return script_path @classmethod @@ -35,29 +62,84 @@ def load_from_cli(cls, cli: CLI) -> None: @classmethod def unload_cli(cls, cli_name: str) -> None: + """Remove the launcher and the module of a loaded CLI: both, or neither. + + Raises: + ValueError: `cli_name` is not a CLI name. + LoaderError: the launcher is not the one cliffy wrote (edited, replaced, a link, unreadable), + or a file could not be deleted. The CLI is left as it was and still runs. + """ script_path = cls.get_cli_script_path(cli_name) - if cls.is_cli_script(script_path): + module_path = cls.get_cli_path(cli_name) + + # checked right before the file is deleted + state = cls.script_state(script_path, cli_name) + if state == "foreign": + raise LoaderError( + f"{script_path} was not created by cliffy, or was changed since. Nothing was removed and " + f"{cli_name} is still loaded. Move or delete that file, then remove {cli_name} again" + ) + try: + if state == "owned": + os.unlink(script_path) + except FileNotFoundError: + state = "missing" + except OSError as e: + raise LoaderError(f"could not delete {script_path}: {e}. Nothing was removed") from e + + try: with contextlib.suppress(FileNotFoundError): - os.remove(script_path) - with contextlib.suppress(FileNotFoundError): - os.remove(cls.get_cli_path(cli_name)) + os.unlink(module_path) + except OSError as e: + if state == "owned": + # put back, so that the CLI is not left with a module and nothing to start it + with contextlib.suppress(OSError): + write_atomically(script_path, cls.get_cli_script(cli_name), executable=True, replace=False) + raise LoaderError(f"could not delete {module_path}: {e}. {cli_name} is still loaded") from e @staticmethod - def is_cli_script(script_path: str) -> bool: - """Whether the file is an entry script written by `deploy_script`""" + def script_state(script_path: str, cli_name: str) -> ScriptState: + """Whether the launcher of a CLI is missing, is the file `deploy_script` writes for it, or is anything else. + + "owned" is a regular file, not a link to one, whose text is the launcher of exactly this CLI. + Only its `#!` line may differ: one environment can be reached through several interpreter paths. + """ try: - with open(script_path, encoding="utf-8") as script: - return "from cliffy.clis." in script.read(4096) + if not stat.S_ISREG(os.lstat(script_path).st_mode): + return "foreign" + # O_NOFOLLOW: a link put there after the lstat fails to open instead of being followed + descriptor = os.open(script_path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)) + except FileNotFoundError: + return "missing" + except OSError: + return "foreign" + + expected = Loader.get_cli_script(cli_name) + try: + with os.fdopen(descriptor, "rb") as script: + if not stat.S_ISREG(os.fstat(script.fileno()).st_mode): + return "foreign" + # more than a launcher holds is not a launcher: the rest does not need to be read + content = script.read(len(expected.encode("utf-8")) + 4096).decode("utf-8") except (OSError, UnicodeDecodeError): - return False + return "foreign" + + interpreter_line, _, body = content.replace("\r\n", "\n").partition("\n") + owned = interpreter_line.startswith("#!") and body == expected.partition("\n")[2] + return "owned" if owned else "foreign" + + @staticmethod + def is_cli_script(script_path: str, cli_name: str) -> bool: + """Whether the file is the launcher written by `deploy_script` for this CLI""" + return Loader.script_state(script_path, cli_name) == "owned" @staticmethod def get_cli_path(cli_name: str) -> str: - return f"{CLIFFY_CLI_DIR}/{cli_name.replace('-', '_')}.py" + return f"{CLIFFY_CLI_DIR}/{validate_cli_name(cli_name).replace('-', '_')}.py" @staticmethod def get_cli_script_path(cli_name: str) -> str: - return f"{PYTHON_BIN}/{cli_name}" + return f"{PYTHON_BIN}/{validate_cli_name(cli_name)}" @staticmethod def get_cli_script(cli_name: str) -> str: diff --git a/tests/test_helper.py b/tests/test_helper.py index 53ce9ec..b24bd9d 100644 --- a/tests/test_helper.py +++ b/tests/test_helper.py @@ -15,6 +15,7 @@ TEMP_FILES, age_datetime, check_requirement, + cli_name_problem, delete_temp_files, exit_err, get_installed_package_versions, @@ -25,6 +26,7 @@ out_err, wrap_as_comment, wrap_as_var, + write_atomically, write_to_file, ) @@ -290,3 +292,44 @@ def test_constants(): assert "Scripts" in PYTHON_BIN else: assert "bin" in PYTHON_BIN + + +def test_write_atomically_replaces_a_link_instead_of_writing_through_it(tmp_path): + target = tmp_path / "target" + target.write_text("untouched") + link = tmp_path / "link" + try: + link.symlink_to(target) + except (OSError, NotImplementedError) as e: + pytest.skip(f"symbolic links cannot be created here: {e}") + + write_atomically(str(link), "new") + + assert target.read_text() == "untouched" + assert not link.is_symlink() and link.read_text() == "new" + assert sorted(path.name for path in tmp_path.iterdir()) == ["link", "target"] + + +def test_write_atomically_without_replace_keeps_an_existing_file(tmp_path): + existing = tmp_path / "existing" + existing.write_text("kept") + + with pytest.raises(FileExistsError): + write_atomically(str(existing), "new", replace=False) + + assert existing.read_text() == "kept" + assert [path.name for path in tmp_path.iterdir()] == ["existing"] + + write_atomically(str(tmp_path / "sub" / "created"), "new", executable=True, replace=False) + assert (tmp_path / "sub" / "created").read_text() == "new" + assert os.access(tmp_path / "sub" / "created", os.X_OK) + + +@pytest.mark.parametrize("name", ["hello", "my-cli", "_x1"]) +def test_cli_name_problem_accepts_names(name): + assert cli_name_problem(name) is None + + +@pytest.mark.parametrize("name", ["", ".", "..", "../x", "a/b", "a\\b", "-x", "a b", "a.b", "*", "x\n", None]) +def test_cli_name_problem_rejects_paths_and_patterns(name): + assert "not a valid CLI name" in (cli_name_problem(name) or "") diff --git a/tests/test_homer.py b/tests/test_homer.py index f320fce..4d35dd6 100644 --- a/tests/test_homer.py +++ b/tests/test_homer.py @@ -258,3 +258,29 @@ def test_corrupt_metadata_can_be_removed(metadata_dir: Path, command: str): assert result.exit_code == 0, result.output assert not Path(metadata_path).exists() assert list(get_clis()) == [] + + +@pytest.mark.parametrize("name", ["../first", "*/../first", "first/../first", "..", "a/b"]) +def test_metadata_is_not_looked_up_or_removed_by_a_path(metadata_dir: Path, name: str): + # `cli rm ../x` used to select /x.json and delete x.* next to it + save_cli(metadata_dir, "first") + stray = metadata_dir / "first.json" + stray.write_text(Path(str(get_metadata_path("first"))).read_text()) + + assert get_metadata_path(name) is None + assert get_metadata(name) is None + remove_metadata(name) + + assert stray.exists() + assert [cli.cli_name for cli in get_clis()] == ["first"] + + +def test_listed_name_is_the_name_of_the_record_file(metadata_dir: Path): + # the name inside an edited record must not decide which CLI `cli rm-all` removes + save_cli(metadata_dir, "first") + metadata_path = Path(str(get_metadata_path("first"))) + record = json.loads(metadata_path.read_text()) + record["cli_name"] = "../../somewhere-else" + metadata_path.write_text(json.dumps(record)) + + assert [cli.cli_name for cli in get_clis()] == ["first"] diff --git a/tests/test_loader.py b/tests/test_loader.py index 345101a..2b15005 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -2,28 +2,51 @@ from pathlib import Path import pytest +from click.testing import CliRunner +from cliffy.cli import cli as cliffy_cli from cliffy.commander import CLI -from cliffy.loader import Loader +from cliffy.homer import get_clis, get_metadata, save_metadata +from cliffy.loader import Loader, LoaderError @pytest.fixture def loader_dirs(monkeypatch, tmp_path): bin_dir = tmp_path / "bin" clis_dir = tmp_path / "clis" + metadata_dir = tmp_path / "metadata" bin_dir.mkdir() clis_dir.mkdir() + metadata_dir.mkdir() monkeypatch.setattr("cliffy.loader.PYTHON_BIN", str(bin_dir)) monkeypatch.setattr("cliffy.loader.CLIFFY_CLI_DIR", clis_dir) + monkeypatch.setattr("cliffy.homer.CLIFFY_METADATA_DIR", metadata_dir) return bin_dir, clis_dir +def load(tmp_path: Path, name: str, code: str = "cli = print") -> None: + """What `cli load` does for a manifest: the launcher, the module and the metadata record.""" + manifest_path = tmp_path / f"{name}.yaml" + manifest_path.write_text(f"name: {name}\n") + cli = CLI(name=name, version="1", code=code) + Loader.load_from_cli(cli) + save_metadata(str(manifest_path), cli) + + +def symlink_or_skip(link: Path, target: Path) -> None: + try: + link.symlink_to(target) + except (OSError, NotImplementedError) as e: + # i.e. Windows without the privilege to create symbolic links + pytest.skip(f"symbolic links cannot be created here: {e}") + + def test_load_does_not_overwrite_foreign_executable(loader_dirs, capsys): bin_dir, clis_dir = loader_dirs # a manifest named `python` or `pip` would land on the interpreter's own executables real_python = bin_dir / "python3.x" real_python.write_bytes(b"\xcf\xfa\xed\xfe real interpreter") - (bin_dir / "python").symlink_to(real_python) + symlink_or_skip(bin_dir / "python", real_python) with pytest.raises(SystemExit): Loader.load_from_cli(CLI(name="python", version="1", code="cli = print")) @@ -34,6 +57,56 @@ def test_load_does_not_overwrite_foreign_executable(loader_dirs, capsys): assert "was not created by cliffy" in capsys.readouterr().err +@pytest.mark.parametrize( + "content", + [ + # a script of someone else that happens to import a loaded CLI + "#!/usr/bin/env python3\nfrom cliffy.clis.other import cli\ncli(['--version'])\n", + # the launcher of another CLI, i.e. a copy + Loader.get_cli_script("other"), + # cliffy's own launcher, edited by hand + Loader.get_cli_script("tool") + "\nprint('my change')\n", + ], + ids=["imports_a_loaded_cli", "launcher_of_another_cli", "edited_launcher"], +) +def test_load_does_not_overwrite_script_it_did_not_write(loader_dirs, capsys, content): + bin_dir, clis_dir = loader_dirs + (bin_dir / "tool").write_text(content) + + with pytest.raises(SystemExit): + Loader.load_from_cli(CLI(name="tool", version="1", code="cli = print")) + + assert (bin_dir / "tool").read_text() == content + assert not (clis_dir / "tool.py").exists() + assert "was not created by cliffy" in capsys.readouterr().err + + +def test_load_does_not_follow_link_to_a_launcher(loader_dirs, tmp_path): + bin_dir, _ = loader_dirs + elsewhere = tmp_path / "elsewhere" + elsewhere.write_text(Loader.get_cli_script("tool")) + symlink_or_skip(bin_dir / "tool", elsewhere) + + with pytest.raises(SystemExit): + Loader.load_from_cli(CLI(name="tool", version="1", code="cli = print")) + + assert (bin_dir / "tool").is_symlink() + assert elsewhere.read_text() == Loader.get_cli_script("tool") + + +def test_load_does_not_replace_file_that_appears_after_the_check(loader_dirs, monkeypatch): + bin_dir, clis_dir = loader_dirs + (bin_dir / "tool").write_text("#!/bin/sh\necho created by someone else, after the check\n") + # the check ran before that file existed + monkeypatch.setattr(Loader, "script_state", staticmethod(lambda script_path, cli_name: "missing")) + + with pytest.raises(SystemExit): + Loader.load_from_cli(CLI(name="tool", version="1", code="cli = print")) + + assert "created by someone else" in (bin_dir / "tool").read_text() + assert not (clis_dir / "tool.py").exists() + + def test_load_replaces_its_own_script(loader_dirs): bin_dir, clis_dir = loader_dirs @@ -43,6 +116,30 @@ def test_load_replaces_its_own_script(loader_dirs): assert "from cliffy.clis.my_cli import cli" in (bin_dir / "my-cli").read_text() assert os.access(bin_dir / "my-cli", os.X_OK) assert (clis_dir / "my_cli.py").read_text() == "cli = repr" + # written to a temporary file next to the target and moved into place + assert sorted(path.name for path in bin_dir.iterdir()) == ["my-cli"] + assert sorted(path.name for path in clis_dir.iterdir()) == ["my_cli.py"] + + +def test_load_replaces_script_written_with_another_interpreter_path(loader_dirs): + bin_dir, _ = loader_dirs + # the same environment reached through another path, i.e. bin/python3 instead of bin/python + body = Loader.get_cli_script("my-cli").split("\n", 1)[1] + (bin_dir / "my-cli").write_text(f"#!/some/venv/bin/python3\n{body}") + + Loader.load_from_cli(CLI(name="my-cli", version="1", code="cli = print")) + + assert (bin_dir / "my-cli").read_text() == Loader.get_cli_script("my-cli") + + +def test_unload_removes_script_and_module(loader_dirs): + bin_dir, clis_dir = loader_dirs + Loader.load_from_cli(CLI(name="my-cli", version="1", code="cli = print")) + + Loader.unload_cli("my-cli") + + assert list(bin_dir.iterdir()) == [] + assert list(clis_dir.iterdir()) == [] def test_unload_removes_module_when_script_is_missing(loader_dirs): @@ -60,6 +157,102 @@ def test_unload_keeps_foreign_executable(loader_dirs): foreign: Path = bin_dir / "pip" foreign.write_text("#!/bin/sh\necho pip\n") - Loader.unload_cli("pip") + with pytest.raises(LoaderError, match="was not created by cliffy"): + Loader.unload_cli("pip") assert foreign.exists() + + +def test_unload_keeps_the_cli_runnable_when_its_script_was_edited(loader_dirs): + bin_dir, clis_dir = loader_dirs + Loader.load_from_cli(CLI(name="my-cli", version="1", code="cli = print")) + edited = (bin_dir / "my-cli").read_text() + "\nprint('my change')\n" + (bin_dir / "my-cli").write_text(edited) + + with pytest.raises(LoaderError): + Loader.unload_cli("my-cli") + + # the module used to be deleted, which left a script that failed with ModuleNotFoundError + assert (bin_dir / "my-cli").read_text() == edited + assert (clis_dir / "my_cli.py").read_text() == "cli = print" + + +def test_unload_puts_the_script_back_when_the_module_cannot_be_removed(loader_dirs, monkeypatch): + bin_dir, clis_dir = loader_dirs + Loader.load_from_cli(CLI(name="my-cli", version="1", code="cli = print")) + real_unlink = os.unlink + + def unlink(path, *args, **kwargs): + if str(path).endswith("my_cli.py"): + raise PermissionError(13, "Permission denied", str(path)) + real_unlink(path, *args, **kwargs) + + monkeypatch.setattr("cliffy.loader.os.unlink", unlink) + + with pytest.raises(LoaderError, match="Permission denied"): + Loader.unload_cli("my-cli") + + assert (bin_dir / "my-cli").read_text() == Loader.get_cli_script("my-cli") + assert os.access(bin_dir / "my-cli", os.X_OK) + assert (clis_dir / "my_cli.py").exists() + + +@pytest.mark.parametrize("command", ["rm", "rm-all"]) +def test_rm_leaves_nothing_half_removed_when_the_script_was_edited(loader_dirs, tmp_path, command): + bin_dir, clis_dir = loader_dirs + load(tmp_path, "kept") + (bin_dir / "kept").write_text("#!/bin/sh\necho mine now\n") + + result = CliRunner().invoke(cliffy_cli, [command] if command == "rm-all" else [command, "kept"]) + + assert result.exit_code == 1, result.output + assert "was not created by cliffy" in result.output + # still listed, and its module is still there: the metadata used to be removed first + assert [cli.cli_name for cli in get_clis()] == ["kept"] + assert (clis_dir / "kept.py").exists() + assert (bin_dir / "kept").read_text() == "#!/bin/sh\necho mine now\n" + + +def test_rm_removes_script_module_and_metadata(loader_dirs, tmp_path): + bin_dir, clis_dir = loader_dirs + load(tmp_path, "gone") + + result = CliRunner().invoke(cliffy_cli, ["rm", "gone"]) + + assert result.exit_code == 0, result.output + assert list(bin_dir.iterdir()) == [] and list(clis_dir.iterdir()) == [] + assert get_metadata("gone") is None + + +@pytest.mark.parametrize("name", ["../outside", "../../outside", "sub/outside", "..", ".", "a\\b", "", "-x", "a b"]) +def test_names_that_are_paths_are_not_cli_names(loader_dirs, name): + with pytest.raises(ValueError, match="not a valid CLI name"): + Loader.get_cli_path(name) + with pytest.raises(ValueError, match="not a valid CLI name"): + Loader.get_cli_script_path(name) + with pytest.raises(ValueError, match="not a valid CLI name"): + Loader.unload_cli(name) + + +@pytest.mark.parametrize("command", ["rm", "update", "info", "build", "docs"]) +@pytest.mark.parametrize("name", ["../outside", "../../outside"]) +def test_commands_do_not_resolve_a_name_outside_of_cliffys_directories(loader_dirs, tmp_path, command, name): + load(tmp_path, "inside") + # what a name with `..` in it would have reached: files one and two levels above the directories + # that hold the launchers, the modules and the metadata records + record = next((tmp_path / "metadata").glob("*/inside.json")).read_text() + outside = [tmp_path / "outside", tmp_path / "outside.py", tmp_path / "outside.json"] + outside += [tmp_path / "metadata" / "outside.json", tmp_path.parent / "outside.json"] + for path in outside: + path.write_text(Loader.get_cli_script("outside") if path.suffix == "" else record) + + try: + result = CliRunner().invoke(cliffy_cli, [command, name]) + + assert result.exit_code == 1, result.output + assert "not a valid CLI name" in result.output + for path in outside: + assert path.exists(), path + finally: + (tmp_path.parent / "outside.json").unlink(missing_ok=True) + assert [cli.cli_name for cli in get_clis()] == ["inside"] From 442030b2875999248d8915df00db1a5fa7002fad Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 16:50:19 -0400 Subject: [PATCH 03/12] Give every run and test run its own temporary directory run_cli and Tester appended their file to a process-wide TEMP_FILES list and then deleted everything on it, so a run that finished removed the file a run on another thread was about to import. The generated code is now imported from a TemporaryDirectory that belongs to the call. TEMP_FILES and delete_temp_files are gone; their tests are replaced by tests of import_generated_code. Co-Authored-By: Claude Opus 5.5 --- cliffy/builder.py | 20 ++++---------------- cliffy/helper.py | 22 ++++++++++++++-------- cliffy/tester.py | 18 +++++------------- tests/test_builder.py | 40 +++++++++++++++++++++++++++++++++++++--- tests/test_helper.py | 31 +++++++++++++++++-------------- tests/test_tester.py | 4 ++-- 6 files changed, 79 insertions(+), 56 deletions(-) diff --git a/cliffy/builder.py b/cliffy/builder.py index 928a873..b4b9994 100644 --- a/cliffy/builder.py +++ b/cliffy/builder.py @@ -13,7 +13,7 @@ from importlib import metadata from io import TextIOWrapper from pathlib import Path -from tempfile import NamedTemporaryFile, TemporaryDirectory +from tempfile import TemporaryDirectory from typing import ClassVar import pybash @@ -24,7 +24,7 @@ from shiv import cli as shiv_cli from cliffy.commander import CLI, SHELL_RUNTIME_IMPORT -from cliffy.helper import TEMP_FILES, delete_temp_files, import_module_from_path, make_executable +from cliffy.helper import import_generated_code, make_executable from cliffy.transformer import Transformer DEFAULT_TARGET = "zipapp" @@ -540,18 +540,6 @@ def get_python_guard(cli_name: str, requires_python: str) -> str: def run_cli(cli_name: str, script_code: str, args: tuple) -> None: - with NamedTemporaryFile( - mode="w", prefix=f"{cli_name}_", suffix=".py", delete=False, encoding="utf-8" - ) as runner_file: - TEMP_FILES.append(runner_file) - try: - runner_file.write(script_code) - runner_file.flush() - module = import_module_from_path(runner_file.name) - finally: - # also when the generated code fails to import - delete_temp_files() - - runner_argvs = [runner_file.name, *list(args)] - sys.argv = runner_argvs + module, module_path = import_generated_code(script_code, f"{cli_name.replace('-', '_')}_cli") + sys.argv = [module_path, *list(args)] module.cli() diff --git a/cliffy/helper.py b/cliffy/helper.py index aa4e056..2d65f73 100644 --- a/cliffy/helper.py +++ b/cliffy/helper.py @@ -12,7 +12,7 @@ from importlib.resources import files from importlib.util import module_from_spec, spec_from_file_location from pathlib import Path -from tempfile import _TemporaryFileWrapper +from tempfile import TemporaryDirectory from types import ModuleType from typing import IO, Any, NoReturn @@ -48,7 +48,6 @@ } ) CLI_NAME_PATTERN = re.compile(r"[A-Za-z_][A-Za-z0-9_-]*") -TEMP_FILES: list[_TemporaryFileWrapper] = [] class ManifestOrCLI(File): @@ -159,12 +158,19 @@ def make_executable(path: str) -> None: os.chmod(path, mode) -def delete_temp_files() -> None: - for file in TEMP_FILES: - with contextlib.suppress(Exception): - file.close() - os.unlink(file.name) - TEMP_FILES.clear() +def import_generated_code(code: str, module_name: str) -> tuple[ModuleType, str]: + """Import generated code from a file, as the module it is when it is loaded. + + The file is in a temporary directory that belongs to this call and is removed before it returns, + also when the import fails. Runs on other threads have their own and cannot remove this one. + + Returns: + The module, and the path it was imported from: the file name in its tracebacks. + """ + with TemporaryDirectory(prefix="cliffy_") as directory: + path = os.path.join(directory, f"{module_name}.py") + Path(path).write_text(code, encoding="utf-8") + return import_module_from_path(path), path def indent_block(block: str, spaces: int = 4) -> str: diff --git a/cliffy/tester.py b/cliffy/tester.py index 7b7d5ee..55168cc 100644 --- a/cliffy/tester.py +++ b/cliffy/tester.py @@ -1,6 +1,5 @@ import inspect from collections.abc import Generator -from tempfile import NamedTemporaryFile from typing import cast import pybash @@ -10,7 +9,7 @@ from pydantic import BaseModel, field_validator from typer.testing import CliRunner -from cliffy.helper import TEMP_FILES, delete_temp_files, import_module_from_path +from cliffy.helper import import_generated_code from cliffy.manifest import CLIManifest, Command, RunBlock from cliffy.parser import Parser from cliffy.transformer import Transformer @@ -42,17 +41,10 @@ def __init__(self, manifest_path: str, blueprint: str | None = None) -> None: with open(manifest_path, encoding="utf-8") as manifest_io: self.T = Transformer(manifest_io, blueprint=blueprint) - with NamedTemporaryFile( - mode="w", prefix=f"{self.T.cli.name}_test_", suffix=".py", delete=False, encoding="utf-8" - ) as runner_file: - TEMP_FILES.append(runner_file) - try: - runner_file.write(self.T.cli.code) - runner_file.flush() - self.module = import_module_from_path(runner_file.name) - finally: - # also when the generated code fails to import - delete_temp_files() + # module_path is the file name of the generated code in tracebacks. The file itself is gone + self.module, self.module_path = import_generated_code( + self.T.cli.code, f"{self.T.cli.name.replace('-', '_')}_test" + ) self.module_funcs = inspect.getmembers(self.module, inspect.isfunction) diff --git a/tests/test_builder.py b/tests/test_builder.py index 840024e..dab5a6c 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -4,6 +4,7 @@ import subprocess import sys import tempfile +import threading import zipfile from importlib import metadata from pathlib import Path @@ -12,6 +13,7 @@ import pytest from click.testing import CliRunner +import cliffy.helper as IMPORT_OWNER from cliffy.builder import ( build_cli, build_cli_from_manifest, @@ -21,7 +23,6 @@ run_cli, ) from cliffy.cli import cli -from cliffy.helper import TEMP_FILES from cliffy.transformer import Transformer EXAMPLES = Path(__file__).parent.parent / "examples" / "blueprints" @@ -310,7 +311,41 @@ def test_run_cli_removes_temp_file_when_import_fails(temp_dir): run_cli("broken", "import no_such_module_for_cliffy", ()) assert glob.glob(f"{temp_dir}/broken_*") == [] - assert TEMP_FILES == [] + # the run has a temporary directory of its own: neither the file nor the directory is left behind + assert list(temp_dir.iterdir()) == [] + + +def test_concurrent_runs_keep_their_own_temp_files(temp_dir, monkeypatch): + # every run cleaned up a process-wide list of temp files: a run that finished deleted the file + # another run, i.e. a reload of `cli dev --run-cli` on another thread, was about to import + first_is_importing, second_is_done = threading.Event(), threading.Event() + real_import = IMPORT_OWNER.import_module_from_path + + def slow_import(path): + if os.path.basename(path).startswith("first"): + first_is_importing.set() + assert second_is_done.wait(10) + return real_import(path) + + monkeypatch.setattr(IMPORT_OWNER, "import_module_from_path", slow_import) + monkeypatch.setattr(sys, "argv", list(sys.argv)) + errors: list[BaseException] = [] + + def first_run(): + try: + run_cli("first", "cli = lambda: None", ()) + except BaseException as e: + errors.append(e) + + thread = threading.Thread(target=first_run) + thread.start() + assert first_is_importing.wait(10) + run_cli("second", "cli = lambda: None", ()) + second_is_done.set() + thread.join(10) + + assert errors == [] + assert list(temp_dir.iterdir()) == [] def test_build_removes_temp_file_when_build_fails(temp_dir, tmp_path): @@ -327,7 +362,6 @@ def test_build_removes_temp_file_when_build_fails(temp_dir, tmp_path): assert glob.glob(f"{temp_dir}/buildme_*") == [] # the script is written into a temporary directory: neither it nor the directory is left behind assert list(temp_dir.iterdir()) == [] - assert TEMP_FILES == [] @pytest.mark.parametrize( diff --git a/tests/test_helper.py b/tests/test_helper.py index b24bd9d..49ac7e6 100644 --- a/tests/test_helper.py +++ b/tests/test_helper.py @@ -2,7 +2,6 @@ import platform from datetime import datetime, timedelta from pathlib import Path -from tempfile import NamedTemporaryFile from unittest.mock import MagicMock, patch import pytest @@ -12,13 +11,12 @@ CLIFFY_METADATA_DIR, PYTHON_BIN, PYTHON_EXECUTABLE, - TEMP_FILES, age_datetime, check_requirement, cli_name_problem, - delete_temp_files, exit_err, get_installed_package_versions, + import_generated_code, import_module_from_path, indent_block, make_executable, @@ -98,18 +96,24 @@ def test_make_executable(tmpdir): assert os.access(str(temp_file), os.X_OK) -# Test for delete_temp_files -def test_delete_temp_files(): - # Arrange - temp_file = NamedTemporaryFile(delete=False) # noqa: SIM115 - TEMP_FILES.append(temp_file) +def test_import_generated_code_removes_its_file(tmp_path, monkeypatch): + monkeypatch.setattr("tempfile.tempdir", str(tmp_path)) - # Act - delete_temp_files() + module, module_path = import_generated_code("answer = 42", "generated_cli") - # Assert - assert not Path(temp_file.name).exists() - assert TEMP_FILES == [] + assert module.answer == 42 + assert module.__name__ == "generated_cli" and os.path.basename(module_path) == "generated_cli.py" + assert not os.path.exists(module_path) + assert list(tmp_path.iterdir()) == [] + + +def test_import_generated_code_removes_its_file_when_the_import_fails(tmp_path, monkeypatch): + monkeypatch.setattr("tempfile.tempdir", str(tmp_path)) + + with pytest.raises(ImportError): + import_generated_code("import no_such_module_for_cliffy", "generated_cli") + + assert list(tmp_path.iterdir()) == [] # Parametrized tests for indent_block @@ -286,7 +290,6 @@ def test_constants(): assert isinstance(CLIFFY_METADATA_DIR, Path) assert isinstance(PYTHON_BIN, str) assert isinstance(PYTHON_EXECUTABLE, str) - assert isinstance(TEMP_FILES, list) if platform.system() == "Windows": assert "Scripts" in PYTHON_BIN diff --git a/tests/test_tester.py b/tests/test_tester.py index 0e8fb97..c851d13 100644 --- a/tests/test_tester.py +++ b/tests/test_tester.py @@ -7,7 +7,6 @@ from pybash.transformer import transform as transform_bash from cliffy.cli import test_command as manifest_test_command -from cliffy.helper import TEMP_FILES from cliffy.tester import ShellScript from cliffy.tester import Tester as ManifestTester @@ -35,7 +34,8 @@ def test_tester_removes_temp_file_when_import_fails(monkeypatch, tmp_path): ManifestTester(str(manifest_path)) assert glob.glob(f"{temp_dir}/broken_*") == [] - assert TEMP_FILES == [] + # the run has a temporary directory of its own: neither the file nor the directory is left behind + assert list(temp_dir.iterdir()) == [] def test_manifest_tests_run_for_click_cli(tmp_path): From 6605710d9629f02f7f7aa66fedff66d006cca5e2 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 16:52:30 -0400 Subject: [PATCH 04/12] Run cli dev reloads one at a time so that the last change wins Every change event started a reload thread of its own. A reload that had read the older manifest could finish last and load the older CLI over the newer one. Reloads now hold one lock and read the manifest only once it is their turn; a reload that waits behind one for a newer change is skipped. The CLI of --run-cli runs outside the lock. An event within a second of the last reload is still treated as a duplicate report of the same save, but only when the watched files are unchanged: a second save within that second used to be dropped. Co-Authored-By: Claude Opus 5.5 --- cliffy/reloader.py | 75 +++++++++++++++++++++++++++----- tests/test_reloader.py | 99 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 10 deletions(-) diff --git a/cliffy/reloader.py b/cliffy/reloader.py index d1ee31b..82b27b9 100644 --- a/cliffy/reloader.py +++ b/cliffy/reloader.py @@ -1,3 +1,4 @@ +import itertools import os import threading import time @@ -13,6 +14,15 @@ from cliffy.loader import Loader from cliffy.transformer import Transformer, source_files +# One reload at a time, for all manifests: a reload reads the manifest and writes the launcher, the module +# and the metadata record, and two of them must not interleave. +RELOAD_LOCK = threading.Lock() +GENERATIONS = itertools.count(1) +# manifest path -> generation of the newest change a reload was started for +NEWEST_GENERATION: dict[str, int] = {} + +FileSignature = tuple[int, int] | None + class Reloader(FileSystemEventHandler): def __init__(self, manifest_path: str, run_cli: bool, run_cli_args: tuple, blueprint: str | None = None) -> None: @@ -26,6 +36,8 @@ def __init__(self, manifest_path: str, run_cli: bool, run_cli_args: tuple, bluep self.scheduled_directories: set[str] = set() # the manifest, the manifests it includes and the Python file of its blueprint self.watched_files: set[str] = set() + # what the watched files looked like at the last change a reload was started for + self.reloaded_signature: dict[str, FileSignature] | None = None self.refresh_watched_files() super().__init__() @@ -49,16 +61,21 @@ def on_modified(self, event: FileSystemEvent) -> None: if not isinstance(event, FileModifiedEvent): return - if datetime.now() - self.last_modified < timedelta(seconds=1): + # A save is reported more than once. Within a second of the last reload an event is only a new + # change when a watched file differs from what that reload was started for. + signature = self.watched_signature() + if datetime.now() - self.last_modified < timedelta(seconds=1) and signature == self.reloaded_signature: return self.last_modified = datetime.now() self.refresh_watched_files() + self.reloaded_signature = self.watched_signature() + generation = next(GENERATIONS) t = threading.Thread( target=self.reload, args=(self.manifest_path, self.run_cli, self.run_cli_args), - kwargs={"blueprint": self.blueprint}, + kwargs={"blueprint": self.blueprint, "generation": generation}, daemon=True, ) try: @@ -66,6 +83,20 @@ def on_modified(self, event: FileSystemEvent) -> None: except RuntimeError as e: # Log the error or handle it as needed out_err(f"Failed to start thread: {e}") + return + # only once its reload runs: reloads of older changes that are still waiting can then be skipped + NEWEST_GENERATION[self.manifest_realpath] = max(generation, NEWEST_GENERATION.get(self.manifest_realpath, 0)) + + def watched_signature(self) -> dict[str, FileSignature]: + """Modification time and size of every watched file, None for one that cannot be read.""" + signature: dict[str, FileSignature] = {} + for path in sorted(self.watched_files): + try: + status = os.stat(path) + signature[path] = (status.st_mtime_ns, status.st_size) + except OSError: + signature[path] = None + return signature @classmethod def watch(cls, manifest_path: str, run_cli: bool, run_cli_args: tuple, blueprint: str | None = None) -> None: @@ -86,12 +117,36 @@ def watch(cls, manifest_path: str, run_cli: bool, run_cli_args: tuple, blueprint observer.join() @staticmethod - def reload(manifest_path: str, run_cli: bool, run_cli_args: tuple, blueprint: str | None = None) -> None: - with open(manifest_path, encoding="utf-8") as manifest_io: - T = Transformer(manifest_io, blueprint=blueprint) if blueprint else Transformer(manifest_io) - Loader.load_from_cli(T.cli) - save_metadata(manifest_path, T.cli) - out(f"✨ Reloaded {T.cli.name} CLI v{T.cli.version} ✨", fg="green") - - if run_cli: + def reload( + manifest_path: str, + run_cli: bool, + run_cli_args: tuple, + blueprint: str | None = None, + generation: int | None = None, + ) -> None: + """Generate the CLI from the manifest as it is now and load it. + + Reloads run one after the other, and each reads the manifest only once it is its turn: the one + that runs last loads the newest text, whatever order the threads were started in. + + Args: + generation: Number of the change this reload was started for. A reload is skipped when a + reload for a newer change of the same manifest has been started: that one does the work. + """ + + def superseded() -> bool: + newest = NEWEST_GENERATION.get(os.path.realpath(manifest_path), 0) + return generation is not None and generation < newest + + with RELOAD_LOCK: + if superseded(): + return + with open(manifest_path, encoding="utf-8") as manifest_io: + T = Transformer(manifest_io, blueprint=blueprint) if blueprint else Transformer(manifest_io) + Loader.load_from_cli(T.cli) + save_metadata(manifest_path, T.cli) + out(f"✨ Reloaded {T.cli.name} CLI v{T.cli.version} ✨", fg="green") + + # not under the lock: a CLI that keeps running must not hold up the next reload + if run_cli and not superseded(): cli_runner(T.cli.name, T.cli.code, run_cli_args) diff --git a/tests/test_reloader.py b/tests/test_reloader.py index 4f107c0..5b6fbbb 100644 --- a/tests/test_reloader.py +++ b/tests/test_reloader.py @@ -1,4 +1,5 @@ import io +import threading import time from datetime import datetime, timedelta from unittest.mock import MagicMock, patch @@ -481,3 +482,101 @@ def test_watch_schedules_every_watched_directory(mock_sleep, MockObserver, tmp_p scheduled = {call.kwargs["path"] for call in MockObserver.return_value.schedule.call_args_list} assert scheduled == {str(tmp_path.resolve()), str((tmp_path / "bp").resolve())} + + +class FakeTransformer: + """Stands in for the Transformer: the "CLI" is the text of the manifest at the time it was read.""" + + reading = threading.Event() + release_first = threading.Event() + calls = 0 + + def __init__(self, manifest_io, **kwargs): + type(self).calls += 1 + call = type(self).calls + content = manifest_io.read().strip() + self.cli = MagicMock(code=content, version=content) + self.cli.name = "raced" + if call == 1: + # the first reload has read the manifest and is still busy generating + type(self).reading.set() + assert type(self).release_first.wait(10) + + +@pytest.fixture +def fake_transformer(): + FakeTransformer.reading, FakeTransformer.release_first = threading.Event(), threading.Event() + FakeTransformer.calls = 0 + return FakeTransformer + + +@patch("cliffy.reloader.out") +@patch("cliffy.reloader.save_metadata") +@patch("cliffy.reloader.Loader.load_from_cli") +def test_older_reload_does_not_overwrite_a_newer_one(mock_load, mock_save, mock_out, fake_transformer, tmp_path): + # every change started a reload of its own: one that had read the old manifest could load its CLI last + manifest_path = tmp_path / "raced.yaml" + manifest_path.write_text("old") + + with patch("cliffy.reloader.Transformer", fake_transformer): + older = threading.Thread(target=Reloader.reload, args=(str(manifest_path), False, ())) + older.start() + assert fake_transformer.reading.wait(10) + + manifest_path.write_text("new") + newer = threading.Thread(target=Reloader.reload, args=(str(manifest_path), False, ())) + newer.start() + # the newer reload has to wait for the older one. Without that it is done well within this time + newer.join(0.5) + fake_transformer.release_first.set() + older.join(10) + newer.join(10) + + assert not older.is_alive() and not newer.is_alive() + assert [call.args[0].code for call in mock_load.call_args_list][-1] == "new" + assert [call.args[1].code for call in mock_save.call_args_list][-1] == "new" + + +@patch("cliffy.reloader.out") +@patch("cliffy.reloader.save_metadata") +@patch("cliffy.reloader.Loader.load_from_cli") +def test_reloads_queued_behind_a_newer_change_are_skipped(mock_load, mock_save, mock_out, tmp_path): + manifest_path = tmp_path / "queued.yaml" + manifest_path.write_text("name: queued\nversion: '1'\ncommands:\n a: print(1)\n") + reloader = Reloader(str(manifest_path), False, ()) + started: list[dict] = [] + + with patch("threading.Thread") as mock_thread: + for _ in range(3): + reloader.last_modified = datetime.now() - timedelta(seconds=2) + reloader.on_modified(modified(manifest_path)) + started = [call.kwargs for call in mock_thread.call_args_list] + + # three changes were accepted before any of their reloads got to run + assert len(started) == 3 + for thread in started: + thread["target"](*thread["args"], **thread["kwargs"]) + + # the last one reads the manifest as it is now: the two before it have nothing left to do + assert mock_load.call_count == 1 + assert mock_save.call_count == 1 + + +def test_a_second_save_within_the_debounce_time_is_not_lost(tmp_path): + # an event within a second of the last one was dropped as a duplicate, also when the file had + # changed again: the CLI then stayed on the older text until the next save + manifest_path = tmp_path / "quick.yaml" + manifest_path.write_text("name: quick\nversion: '1'\ncommands:\n a: print(1)\n") + reloader = Reloader(str(manifest_path), False, ()) + reloader.last_modified = datetime.now() - timedelta(seconds=2) + + with patch("threading.Thread") as mock_thread: + reloader.on_modified(modified(manifest_path)) + # the same save reported twice, as file system watchers do + reloader.on_modified(modified(manifest_path)) + assert mock_thread.call_count == 1 + + manifest_path.write_text("name: quick\nversion: '2'\ncommands:\n a: print(1)\n b: print(2)\n") + reloader.on_modified(modified(manifest_path)) + reloader.on_modified(modified(manifest_path)) + assert mock_thread.call_count == 2 From b223ccebce1790db0178d461b691952c76034168 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 16:54:03 -0400 Subject: [PATCH 05/12] Print rendered code as it is generated, with the status line on stderr cli render and cli init --render printed through rich with overflow="fold". Lines longer than the terminal, or than 80 columns when redirected, were broken, so cli render x.yaml > x.py was not the module that runs. Redirected output is now the generated text character for character; on a terminal the text is highlighted and left to the terminal to wrap. The 'Rendered ...' line goes to stderr. Co-Authored-By: Claude Opus 5.5 --- cliffy/cli.py | 26 ++++++++++++++---- cliffy/rich.py | 2 +- tests/test_cli.py | 67 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 6 deletions(-) diff --git a/cliffy/cli.py b/cliffy/cli.py index 2584cf6..8ca2525 100644 --- a/cliffy/cli.py +++ b/cliffy/cli.py @@ -59,6 +59,23 @@ def show_aliases_callback(ctx: Any, param: Any, val: bool) -> None: ctx.exit() +def stdout_is_terminal() -> bool: + return sys.stdout.isatty() + + +def print_source(text: str) -> None: + """Print generated code or a manifest to stdout as it is: no line is broken and nothing is added. + + Redirected or piped, the output is the text itself, character for character. On a terminal the + text is highlighted and long lines are left to the terminal to wrap. + """ + if stdout_is_terminal(): + Console().print(text, soft_wrap=True, emoji=False, markup=False, end="" if text.endswith("\n") else "\n") + else: + # color=True: nothing that looks like an escape sequence is removed from the text + click.echo(text, nl=not text.endswith("\n"), color=True) + + blueprint_option = click.option( "--blueprint", "-b", @@ -119,9 +136,9 @@ def update(cli_names: list[str]) -> None: def render(manifest: TextIO, blueprint: str | None = None) -> None: """Render the CLI manifest generation as code""" T = Transformer(manifest, blueprint=blueprint) - console = Console() - console.print(T.cli.code, overflow="fold", emoji=False, markup=False) - out(f"# Rendered {T.cli.name} CLI v{T.cli.version} ~", fg="green") + print_source(T.cli.code) + # not on stdout: `cli render x.yaml > x.py` writes the module and nothing else + out(f"# Rendered {T.cli.name} CLI v{T.cli.version} ~", fg="green", err=True) @click.argument("manifest", type=click.File("rb")) @@ -166,8 +183,7 @@ def init(cli_name: str, render: bool, raw: bool, json_schema: bool) -> None: exit_err(f"~ {e}") if render: - console = Console() - console.print(template, overflow="fold", emoji=False, markup=False) + print_source(template) else: try: write_to_file(f"{cli_name}.yaml", text=template) diff --git a/cliffy/rich.py b/cliffy/rich.py index 5d41f91..82c4787 100644 --- a/cliffy/rich.py +++ b/cliffy/rich.py @@ -19,7 +19,7 @@ def print(self, text: Any, **kwargs: Any) -> None: if isinstance(text, PlainTable): click.echo(text) else: - print(text) + print(text, end=kwargs.get("end", "\n")) class PlainTable: diff --git a/tests/test_cli.py b/tests/test_cli.py index dc4353b..83a2c8e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -7,6 +7,7 @@ from pathlib import Path from unittest.mock import MagicMock, patch +import click import pytest from click.testing import CliRunner @@ -28,6 +29,7 @@ from cliffy.homer import get_metadata from cliffy.loader import Loader from cliffy.manifest import CLIMetadata +from cliffy.transformer import Transformer ANSI_RE = re.compile(r"(?:\x1B[@-_]|[\x80-\x9F])[0-?]*[ -/]*[@-~]") @@ -471,3 +473,68 @@ def test_init_template_loads_runs_and_passes_its_own_tests(isolated_fs, flags): tested = runner.invoke(manifest_test_command, ["mycli.yaml"]) assert "All tests passed" in tested.output assert tested.exit_code == 0 + + +LONG_LINE = "print('" + "long line " * 30 + "')" +WIDE_MANIFEST = f"name: wide\nversion: '1'\nhelp: {'wide help ' * 20}\ncommands:\n a: {LONG_LINE}\n" + + +def generated_code(manifest_path) -> str: + with open(manifest_path, encoding="utf-8") as manifest_io: + return Transformer(manifest_io).cli.code + + +def test_render_prints_exactly_the_generated_code(tmp_path): + # the code was printed through rich with overflow="fold": lines longer than the terminal (80 columns + # when redirected) were cut in two, so `cli render x.yaml > x.py` was not the code that runs + manifest_path = tmp_path / "wide.yaml" + manifest_path.write_text(WIDE_MANIFEST) + + result = CliRunner().invoke(render_command, [str(manifest_path)]) + + assert result.exit_code == 0, result.output + assert result.stdout == generated_code(manifest_path) + assert f" {LONG_LINE}" in result.stdout.splitlines() + compile(result.stdout, "rendered", "exec") + # the status line is not part of the code + assert "Rendered wide CLI v1" in result.stderr + assert "Rendered" not in result.stdout + + +def test_render_redirected_to_a_file_is_the_module(tmp_path): + manifest_path = tmp_path / "wide.yaml" + manifest_path.write_text(WIDE_MANIFEST) + rendered_path = tmp_path / "rendered.py" + + with open(rendered_path, "wb") as rendered: + subprocess.run( + [sys.executable, "-m", "cliffy", "render", str(manifest_path)], + stdout=rendered, + stderr=subprocess.PIPE, + check=True, + env={**os.environ, "PYTHONIOENCODING": "utf-8", "COLUMNS": "40", "FORCE_COLOR": "1"}, + ) + + assert rendered_path.read_bytes() == generated_code(manifest_path).encode("utf-8") + ran = subprocess.run([sys.executable, str(rendered_path), "a"], capture_output=True, text=True) + assert ran.returncode == 0, ran.stderr + assert ran.stdout.strip() == ("long line " * 30).strip() + + +def test_render_on_a_terminal_does_not_break_lines(tmp_path, monkeypatch): + manifest_path = tmp_path / "wide.yaml" + manifest_path.write_text(WIDE_MANIFEST) + monkeypatch.setattr("cliffy.cli.stdout_is_terminal", lambda: True) + + result = CliRunner().invoke(render_command, [str(manifest_path)], env={"COLUMNS": "40"}) + + assert result.exit_code == 0, result.output + assert click.unstyle(result.stdout) == generated_code(manifest_path) + + +def test_init_render_prints_the_template_as_it_is_written(isolated_fs): + rendered = CliRunner().invoke(init_command, ["wide", "--render"], env={"COLUMNS": "40"}) + written = CliRunner().invoke(init_command, ["wide"]) + + assert rendered.exit_code == 0 and written.exit_code == 0 + assert rendered.stdout == (isolated_fs / "wide.yaml").read_text() From 382a2707a6ce5cae437958e66bb82bf10e1bc5b9 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 16:58:31 -0400 Subject: [PATCH 06/12] Report cli test errors in the manifest's terms, without tracebacks - A command that raises is reported as the command (or callback) it is in the manifest, with the exception and the line of its run block, instead of ''. cli test --traceback adds the full traceback. - An assertion that raises something other than AssertionError, or is not valid Python, is reported with its line. Both printed a traceback through cliffy/cli.py and cliffy/tester.py. - A manifest whose generated code does not import, and a shell step whose program cannot be started, end the run with an error message instead of a traceback. The generated CLI records which scripts each function was generated from (CLI.blocks), which is what maps a line of generated code to a manifest line. Co-Authored-By: Claude Opus 5.5 --- cliffy/cli.py | 67 +++++++++++------ cliffy/commander.py | 24 +++++- cliffy/commanders/click.py | 6 +- cliffy/commanders/typer.py | 2 + cliffy/tester.py | 116 +++++++++++++++++++++++++++-- tests/test_tester.py | 145 +++++++++++++++++++++++++++++++++++++ 6 files changed, 331 insertions(+), 29 deletions(-) diff --git a/cliffy/cli.py b/cliffy/cli.py index 8ca2525..db9003a 100644 --- a/cliffy/cli.py +++ b/cliffy/cli.py @@ -36,7 +36,7 @@ from cliffy.manifest import CLIManifest from cliffy.reloader import Reloader from cliffy.rich import Console, click, print_rich_table # type: ignore -from cliffy.tester import ShellScript, Tester +from cliffy.tester import ASSERTION_FILE, GeneratedImportError, ShellScript, Tester from cliffy.transformer import Transformer CONTEXT_SETTINGS = {"help_option_names": ["-h", "--help"]} @@ -376,10 +376,20 @@ def dev(manifest: str, run_cli: bool, run_cli_args: tuple[str], blueprint: str | default=False, help="exit instantly on first error or failed test.", ) +@click.option( + "--traceback", + "show_traceback", + is_flag=True, + default=False, + help="Also print the full Python traceback of a command that raises.", +) @blueprint_option -def test(manifest: str, exitfirst: bool, blueprint: str | None = None) -> None: +def test(manifest: str, exitfirst: bool, show_traceback: bool = False, blueprint: str | None = None) -> None: """Run tests defined in a manifest""" - tester = Tester(manifest, blueprint=blueprint) + try: + tester = Tester(manifest, blueprint=blueprint) + except GeneratedImportError as e: + exit_err(f"~ error in {manifest}: the generated code does not work\n {e}") if not tester.test_pipeline: exit_err("Missing tests section in manifest") out("✨ Invoking tests ✨", nl=False) @@ -393,36 +403,49 @@ def test(manifest: str, exitfirst: bool, blueprint: str | None = None) -> None: tester.invoke_shell(case) except SyntaxError as e: exit_err(f"~ error in {manifest}: test step `{case.command}`, column {e.offset}: {e.msg}") + except Exception as e: + # i.e. a program that does not exist + exit_err(f"~ error in {manifest}: test step `{case.command}` could not be run: {type(e).__name__}: {e}") continue case_number += 1 out(f"\n\n🪄 > {tester.T.cli.name} {case.command}\n") + test = tester.invoke_test(case.command, case.assert_script) + result = next(test) + out("⚗️ > \n" + result.output) + if raised := tester.describe_exception(case.command, result): + # the exception of the command, without frames of cliffy or of the generated module + out(raised) + if show_traceback and result.exc_info: + out("".join(traceback.format_exception(*result.exc_info))) + elif isinstance(result.exception, SystemExit) and result.exit_code: + out(f"exit code {result.exit_code}") + try: - test = tester.invoke_test(case.command, case.assert_script) - result = next(test) - out("⚗️ > \n" + result.output) - if result.exception: - out(str(result)) + # the assertions of the test case next(test, "") out(f"\n✅ {case_number} of {total}") passed += 1 - except AssertionError: - _, _, tb = sys.exc_info() - tb_info = traceback.extract_tb(tb) - _, line_no, _, _ = tb_info[-1] - expr = case.assert_script.split("\n")[line_no - 1] - out(f"💔 AssertionError: (line {line_no}) > {expr}") - if exitfirst: - raise SystemExit(1) from None - except SyntaxError: - out("💔 Syntax error") - traceback.print_exc() + except SyntaxError as e: + # the assertion itself is not Python + lines = case.assert_script.split("\n") + line = lines[min(max(e.lineno or 1, 1), len(lines)) - 1].strip() + out(f"💔 the assertion is not valid Python, line {e.lineno}, column {e.offset}: {e.msg}\n {line}") if exitfirst: raise SystemExit(1) from None - except Exception: - out("💔 Exception") - traceback.print_exc() + except Exception as e: + # a failed assertion, or an assertion that raises: reported with its line of the test case + frames = [frame for frame in traceback.extract_tb(e.__traceback__) if frame.filename == ASSERTION_FILE] + line_no = frames[0].lineno if frames else None + lines = case.assert_script.split("\n") + expr = lines[line_no - 1] if line_no and line_no <= len(lines) else case.assert_script.strip() + where = f"(line {line_no}) > {expr}" + out( + f"💔 AssertionError: {where}" + if isinstance(e, AssertionError) + else f"💔 {type(e).__name__}: {e} {where}" + ) if exitfirst: raise SystemExit(1) from None diff --git a/cliffy/commander.py b/cliffy/commander.py index 353b5bd..a188a75 100644 --- a/cliffy/commander.py +++ b/cliffy/commander.py @@ -38,14 +38,23 @@ def callback_label(callback: Callback) -> str: return f"callback of group `{callback.group}`, run" if callback.group else "callback of the CLI, run" +def script_text(script: RunBlock | RunBlockList) -> str: + """A run block as the one script it is generated from""" + return script.to_script() if isinstance(script, RunBlockList) else str(script.root) + + class CLI(BaseModel): name: str version: str code: str requires: list[str] = [] blueprint: str | None = None - # generated function name -> where it comes from in the manifest, i.e. {"db_create": "commands.db.create"} + # generated function name -> where it comes from in the manifest, i.e. {"db_create": "command `db.create`"} functions: dict[str, str] = {} + # generated function name -> the scripts its body is generated from, in order, as (key in the manifest, + # script): {"db_create": [("pre_run", "..."), ("run", "...")]}. Lets a line of the generated code be + # reported as a line of the manifest. + blocks: dict[str, list[tuple[str, str]]] = {} class BaseGroup(BaseModel): @@ -116,6 +125,7 @@ class Commander(ABC): __slots__ = ( "aliases_by_commands", "base_imports", + "blocks", "callbacks_by_group", "cli", "command_functions", @@ -161,6 +171,7 @@ def __init__(self, manifest: CLIManifest) -> None: # nothing else uses: two manifest names can map to the same identifier (`db-app` and the group `db`). self.taken_names: set[str] = set(self.manifest.vars) | self.user_defined_names() self.functions: dict[str, str] = {} + self.blocks: dict[str, list[tuple[str, str]]] = {} self.command_functions: dict[str, str] = {} for group in self.groups.values(): group.var = self.unique_name(to_identifier(group.name + "_app")) @@ -206,8 +217,18 @@ def command_func_name(self, command: Command) -> str: self.command_functions[command.name] = self.unique_name( self.parser.get_command_func_name(command), source=f"command `{command.name}`" ) + self.blocks[self.command_functions[command.name]] = self.command_blocks(command) return self.command_functions[command.name] + @staticmethod + def command_blocks(command: Command) -> list[tuple[str, str]]: + """The scripts of a command in the order `Parser.parse_command_run` writes them.""" + scripts = [("pre_run", command.pre_run), ("run", command.run), ("post_run", command.post_run)] + return [(key, script_text(script)) for key, script in scripts if script or key == "run"] + + def record_callback_blocks(self, function_name: str, callbacks: list[Callback]) -> None: + self.blocks[function_name] = [("run", script_text(callback.run)) for callback in callbacks] + def _merge_command_template(self, command: Command) -> None: """Merge command with its template if specified.""" if not command.template: @@ -492,4 +513,5 @@ def generate_cli(manifest: CLIManifest, commander_cls: type[Commander] = Command code=commander.cli, requires=manifest.requires, functions=commander.functions, + blocks=commander.blocks, ) diff --git a/cliffy/commanders/click.py b/cliffy/commanders/click.py index b9c91e1..ad5cc1c 100644 --- a/cliffy/commanders/click.py +++ b/cliffy/commanders/click.py @@ -67,6 +67,7 @@ def show_aliases(ctx, param, value): """ callbacks = self.callbacks_for() self.functions["cli"] = "callback of the CLI" + self.record_callback_blocks("cli", callbacks) self.cli += """ @click.group(context_settings=CONTEXT_SETTINGS""" if "invoke_without_command" not in self.manifest.cli_options: @@ -117,6 +118,7 @@ def define_groups(self) -> None: parsed_help = clean_help(group.help) callbacks = self.callbacks_for(group) self.functions[group.var_name] = f"callback of group `{group.name}`" + self.record_callback_blocks(group.var_name, callbacks) self.cli += f""" @click.group({self.parse_callback_options(callbacks)}){self.parse_callback_decorators(callbacks)} def {group.var_name}({self.parse_callback_param_names(callbacks)}): @@ -182,10 +184,12 @@ def {self.alias_func_name(command, alias)}({self.click_parser.get_param_names(co def alias_func_name(self, command: Command, alias: str) -> str: """Name of the function that is registered under an alias: Click needs one function per command name.""" - return self.unique_name( + name = self.unique_name( self.click_parser.get_alias_func_name(self.command_func_name(command), alias), source=f"command `{command.name}`", ) + self.blocks[name] = self.command_blocks(command) + return name def add_main_block(self) -> None: self.cli += """ diff --git a/cliffy/commanders/typer.py b/cliffy/commanders/typer.py index 1a15f2a..fb00f11 100644 --- a/cliffy/commanders/typer.py +++ b/cliffy/commanders/typer.py @@ -48,6 +48,7 @@ def aliases_callback(value: bool): """ callbacks = self.callbacks_for() self.functions["main"] = "callback of the CLI" + self.record_callback_blocks("main", callbacks) self.cli += f""" @cli.callback({self.parse_callback_options(callbacks)}) def main({self.parse_callback_params(callbacks)}""" @@ -118,6 +119,7 @@ def define_groups(self) -> None: callback_name = self.unique_name( f"{group.var_name}_callback", source=f"callback of group `{group.name}`" ) + self.record_callback_blocks(callback_name, callbacks) self.cli += f""" @{group.var_name}.callback({self.parse_callback_options(callbacks)}) def {callback_name}({self.parse_callback_params(callbacks)} diff --git a/cliffy/tester.py b/cliffy/tester.py index 55168cc..8851fe1 100644 --- a/cliffy/tester.py +++ b/cliffy/tester.py @@ -1,4 +1,8 @@ +import ast import inspect +import linecache +import os +import traceback from collections.abc import Generator from typing import cast @@ -9,11 +13,22 @@ from pydantic import BaseModel, field_validator from typer.testing import CliRunner +from cliffy.checker import describe from cliffy.helper import import_generated_code from cliffy.manifest import CLIManifest, Command, RunBlock -from cliffy.parser import Parser +from cliffy.parser import Parser, transform_script from cliffy.transformer import Transformer +# File name of the assertions of a test case in tracebacks +ASSERTION_FILE = "" + + +class GeneratedImportError(ImportError): + """The code generated for the manifest fails while it is imported. + + The message names the part of the manifest the failing code was generated from. + """ + class TestCase(BaseModel): command: str @@ -42,9 +57,14 @@ def __init__(self, manifest_path: str, blueprint: str | None = None) -> None: self.T = Transformer(manifest_io, blueprint=blueprint) # module_path is the file name of the generated code in tracebacks. The file itself is gone - self.module, self.module_path = import_generated_code( - self.T.cli.code, f"{self.T.cli.name.replace('-', '_')}_test" - ) + module_name = f"{self.T.cli.name.replace('-', '_')}_test" + try: + self.module, self.module_path = import_generated_code(self.T.cli.code, module_name) + except ImportError as e: + raise GeneratedImportError(self.describe_import_error(e, f"{module_name}.py")) from e + # the lines of a traceback are read from here (`cli test --traceback`) + code_lines = self.T.cli.code.splitlines(keepends=True) + linecache.cache[self.module_path] = (len(self.T.cli.code), None, code_lines, self.module_path) self.module_funcs = inspect.getmembers(self.module, inspect.isfunction) @@ -81,9 +101,95 @@ def invoke_test(self, command: str, script: str) -> Generator[Result, None, None result, stream, unstyle(captured.decode(self.runner.charset, "replace")).encode(self.runner.charset) ) yield result - code = compile(script, f"test_{self.T.cli.name}.py", "exec") + code = compile(script, ASSERTION_FILE, "exec") exec(code, {}, {"result": result, "result_text": result.output.strip()}) + def describe_import_error(self, error: ImportError, file_name: str) -> str: + """Why the generated code does not import, against the part of the manifest it comes from.""" + cause = error.__cause__ or error + if isinstance(cause, SyntaxError): + line_number = cause.lineno + else: + frames = [ + frame + for frame in traceback.extract_tb(cause.__traceback__) + if os.path.basename(frame.filename) == file_name + ] + line_number = frames[-1].lineno if frames else None + return describe(self.T.cli, cause, line_number) + + def describe_exception(self, command: str, result: Result) -> str | None: + """An exception a command ended with, in the manifest's terms. None when it did not raise. + + `command `db.create` raised ValueError: no such table`, then the line of the script it was + raised on as it is written in the manifest. No frame of cliffy, of the framework or of the + generated module is shown. + """ + error = result.exception + if error is None or isinstance(error, SystemExit): + # an exit code is a result of the command, not a failure of it + return None + + trace = result.exc_info[2] if result.exc_info else error.__traceback__ + frames = [frame for frame in traceback.extract_tb(trace) if frame.filename == self.module_path and frame.lineno] + functions = self.T.cli.functions + owner = next((frame for frame in reversed(frames) if frame.name in functions), None) + who = functions[owner.name] if owner else f"`{self.T.cli.name} {command}`" + message = f"💥 {who} raised {type(error).__name__}: {error}" + + code_lines = self.T.cli.code.splitlines() + if owner and owner.lineno: + message += f"\n {self.locate_line(owner.name, owner.lineno)}" + if frames and frames[-1] is not owner and (line_number := frames[-1].lineno): + # raised further in: in a function of the `functions` section, or in a function defined in the script + located = self.locate_line(owner.name, line_number, or_none=True) if owner else None + source = code_lines[line_number - 1].strip() if line_number <= len(code_lines) else "" + message += f"\n {located or f'in `{frames[-1].name}` (functions): {source}'}" + return message + + def locate_line(self, function_name: str, line_number: int, or_none: bool = False) -> str | None: + """Line `line_number` of the generated code as `run, line 2: `. + + Falls back to the generated line itself when it is not part of a script of the function, i.e. + when a blueprint changed the code after it was generated. + """ + code_lines = self.T.cli.code.splitlines() + fallback = None if or_none else code_lines[line_number - 1].strip() + function = next( + ( + node + for node in ast.parse(self.T.cli.code).body + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef) and node.name == function_name + ), + None, + ) + if function is None or not function.lineno <= line_number <= (function.end_lineno or 0): + return fallback + + # the scripts follow each other in the body, each as `Parser.parse_run_block` wrote it + blocks = [] + for key, script in self.T.cli.blocks.get(function_name, []): + transformed = transform_script(script, key) + skipped = (len(transformed) - len(transformed.lstrip())) and transformed[ + : len(transformed) - len(transformed.lstrip()) + ].count("\n") + blocks.append((key, script, skipped, [" " * 4 + line for line in transformed.strip().split("\n")])) + + body = [line for _, _, _, generated in blocks for line in generated] + last = function.end_lineno or function.lineno + start = next( + (first for first in range(function.lineno + 1, last + 1) if code_lines[first - 1 : last] == body), None + ) + if start is None: + return fallback + + for key, script, skipped, generated in blocks: + if line_number < start + len(generated): + script_line = line_number - start + skipped + 1 + return f"{key}, line {script_line}: {script.split(chr(10))[script_line - 1].strip()}" + start += len(generated) + return fallback + def is_valid_command(self, command: str) -> bool: command_name = command.split(" ")[0] cmd = Command(name=command_name, run=RunBlock("")) diff --git a/tests/test_tester.py b/tests/test_tester.py index c851d13..5011449 100644 --- a/tests/test_tester.py +++ b/tests/test_tester.py @@ -135,3 +135,148 @@ def test_assertions_see_output_without_ansi_styles(tmp_path): assert "All tests passed" in result.output, result.output assert result.exit_code == 0 + + +RAISING_MANIFEST = """\ +name: raising +version: '1' +use_click: {use_click} +functions: + - | + def parse(text): + import json + return json.loads(text) +commands: + boom: + help: Raises on its second line + pre_run: prepared = True + run: | + + value = 1 + raise RuntimeError("the user's own problem") + deep: + run: | + print("before") + parse("{{") + quit: raise SystemExit(3) +tests: +{tests} +""" + + +def run_manifest_tests(tmp_path, tests: str, *args: str, use_click: bool = False, manifest: str = RAISING_MANIFEST): + manifest_path = tmp_path / "raising.yaml" + manifest_path.write_text(manifest.format(use_click=str(use_click).lower(), tests=tests)) + return CliRunner().invoke(manifest_test_command, [str(manifest_path), *args]) + + +def assert_no_traceback(output: str) -> None: + """Nothing of cliffy itself and nothing of the generated module is shown: SOUL principle 3""" + assert "Traceback" not in output, output + assert 'File "' not in output, output + for internal in ("tester.py", "cli.py", "helper.py", "_test.py", "site-packages", "`: nothing said where in the manifest it came from + result = run_manifest_tests(tmp_path, " - boom: assert result.exit_code == 0\n", use_click=use_click) + + assert result.exit_code == 1 + assert "command `boom` raised RuntimeError: the user's own problem" in result.output + # line 3 of the run block as it is written in the manifest: the blank first line counts + assert 'run, line 3: raise RuntimeError("the user\'s own problem")' in result.output + assert "AssertionError: (line 1) > assert result.exit_code == 0" in result.output + assert_no_traceback(result.output) + + +@pytest.mark.parametrize("use_click", [False, True], ids=["typer", "click"]) +def test_exception_from_a_helper_names_the_command_and_the_helper(tmp_path, use_click): + result = run_manifest_tests(tmp_path, " - deep: assert result.exit_code == 0\n", use_click=use_click) + + assert result.exit_code == 1 + assert "command `deep` raised JSONDecodeError: " in result.output + assert 'run, line 2: parse("{")' in result.output + assert "in `parse` (functions): return json.loads(text)" in result.output + assert_no_traceback(result.output) + + +def test_expected_exception_can_still_be_asserted_on(tmp_path): + tests = " - boom: assert isinstance(result.exception, RuntimeError)\n - quit: assert result.exit_code == 3\n" + + result = run_manifest_tests(tmp_path, tests) + + assert result.exit_code == 0, result.output + assert "All tests passed" in result.output + assert "command `boom` raised RuntimeError" in result.output + # an exit code is not an exception of the command + assert "command `quit` raised" not in result.output + assert_no_traceback(result.output) + + +def test_traceback_flag_shows_the_traceback_of_the_command(tmp_path): + result = run_manifest_tests(tmp_path, " - boom: assert result.exit_code == 0\n", "--traceback") + + assert result.exit_code == 1 + assert "command `boom` raised RuntimeError: the user's own problem" in result.output + assert "Traceback (most recent call last)" in result.output + # with the lines of the generated code, although its file is gone + assert 'raise RuntimeError("the user\'s own problem")' in result.output.split("Traceback")[1] + + +@pytest.mark.parametrize( + "assertion, problem", + [ + ("assert undefined_name", "NameError: name 'undefined_name' is not defined (line 1) > assert undefined_name"), + ("assert result.output.nope()", "AttributeError: 'str' object has no attribute 'nope' (line 1) > assert"), + ("assert (", "the assertion is not valid Python, line 1, column 8: '(' was never closed"), + ], + ids=["name_error", "attribute_error", "syntax_error"], +) +def test_broken_assertion_is_reported_without_a_traceback(tmp_path, assertion, problem): + # these printed a traceback through cliffy/cli.py and cliffy/tester.py + tests = f" - quit: {assertion}\n - quit: assert result.exit_code == 3\n" + + result = run_manifest_tests(tmp_path, tests) + + assert result.exit_code == 1 + assert problem in result.output + assert "2 of 2" in result.output + assert_no_traceback(result.output) + + +def test_assertion_error_on_a_later_line_is_located(tmp_path): + tests = " - quit: |\n assert result.exit_code == 3\n assert 'nothing' in result.output\n" + + result = run_manifest_tests(tmp_path, tests) + + assert result.exit_code == 1 + assert "AssertionError: (line 2) > assert 'nothing' in result.output" in result.output + assert_no_traceback(result.output) + + +def test_manifest_that_does_not_import_is_reported_against_the_manifest(tmp_path): + # `cli test` ended with a traceback through cliffy/helper.py that named a deleted temp file + manifest_path = tmp_path / "broken.yaml" + manifest_path.write_text(BROKEN_MANIFEST + "tests:\n - a: assert result.exit_code == 0\n") + + result = CliRunner().invoke(manifest_test_command, [str(manifest_path)]) + + assert result.exit_code == 1 + assert isinstance(result.exception, SystemExit) + assert "module level code (`imports`, `vars`, `types` or `functions`)" in result.output + assert "ModuleNotFoundError: No module named 'no_such_module_for_cliffy'" in result.output + assert "import no_such_module_for_cliffy" in result.output + assert_no_traceback(result.output) + + +def test_shell_step_that_cannot_start_ends_the_run_without_a_traceback(tmp_path): + tests = " - $ no-such-program-for-cliffy --flag\n - quit: assert result.exit_code == 3\n" + + result = run_manifest_tests(tmp_path, tests) + + assert result.exit_code == 1 + assert isinstance(result.exception, SystemExit) + assert "test step `$ no-such-program-for-cliffy --flag` could not be run: FileNotFoundError" in result.output + assert_no_traceback(result.output) From 9cf96b6ab248426dc89d031de9a9d1f589639af6 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 17:02:07 -0400 Subject: [PATCH 07/12] Show the manifest's examples in the --help of the generated CLI SOUL principle 7 promises examples in the help, but `examples` was only read by cli docs. Both backends now put an Examples section into the epilog of the CLI: one paragraph per example, `$ command # description`, after an epilog set in cli_options. The text is a string literal in the generated code like every other manifest string, and for Typer brackets are escaped so that rich markup does not drop `[home]` from a command line. A manifest without examples generates the same code as before. examples/hello.yaml and examples/click_hello.yaml carry examples and a test for them. Co-Authored-By: Claude Opus 5.5 --- cliffy/commander.py | 41 +++++++++++ cliffy/commanders/click.py | 4 +- cliffy/commanders/typer.py | 6 +- examples/click_hello.yaml | 7 ++ examples/generated/clickhello.py | 2 +- examples/generated/hello.py | 2 +- examples/hello.yaml | 9 ++- tests/test_codegen.py | 112 +++++++++++++++++++++++++++++++ 8 files changed, 176 insertions(+), 7 deletions(-) diff --git a/cliffy/commander.py b/cliffy/commander.py index a188a75..890f7aa 100644 --- a/cliffy/commander.py +++ b/cliffy/commander.py @@ -6,6 +6,7 @@ from collections import defaultdict from collections.abc import ItemsView, Iterator, ValuesView from contextlib import contextmanager +from typing import Any import pybash from pydantic import BaseModel @@ -326,6 +327,46 @@ def generate_cli(self) -> None: if SHELL_RUNTIME_REFERENCE.search(self.cli, base_imports_end): self.cli = self.cli[:base_imports_end] + SHELL_RUNTIME_IMPORT + self.cli[base_imports_end:] + # A rich markup tag, as rich.markup finds it: `[bold]`, `[/]`, `[#ff0000]`, `[@click]` + MARKUP_TAG = re.compile(r"(\\*)(\[[a-z#/@][^[]*?])") + + def examples_epilog(self, escape_markup: bool = False) -> str: + """Text for the end of the CLI's `--help`: the manifest's `examples`, one paragraph each. + + `$ notes add "buy milk" # Add a note.` A paragraph per example, because the help renderers + (Click, rich-click and Typer) agree on nothing else: they join or re-wrap the lines within one. + + Args: + escape_markup: Typer reads `[...]` in help text as rich markup and would drop `[home]` from + `--tag [home]`. An example is a command line to copy: it is shown as written. + """ + lines = [] + for example in self.manifest.examples: + command, description = ( + (example, None) if isinstance(example, str) else (example.command, example.description) + ) + line = "$ " + " ".join(command.splitlines()).strip() + if description and description.strip(): + line += " # " + " ".join(description.split()) + lines.append(line) + if not lines: + return "" + + epilog = "\n\n".join(["Examples:", *lines]) + if escape_markup: + epilog = self.MARKUP_TAG.sub(lambda tag: f"{tag.group(1) * 2}\\{tag.group(2)}", epilog) + return epilog + + def cli_options_with_examples(self, escape_markup: bool = False) -> dict[str, Any]: + """`cli_options` with the examples in `epilog`, after an epilog the manifest sets itself.""" + options = dict(self.manifest.cli_options) + if examples := self.examples_epilog(escape_markup): + own_epilog = options.get("epilog") + options["epilog"] = ( + f"{own_epilog}\n\n{examples}" if isinstance(own_epilog, str) and own_epilog else examples + ) + return options + def header(self) -> str: """First line of the generated code. It holds nothing that changes between two runs, such as the time: the same manifest gives the same code.""" diff --git a/cliffy/commanders/click.py b/cliffy/commanders/click.py index ad5cc1c..2787580 100644 --- a/cliffy/commanders/click.py +++ b/cliffy/commanders/click.py @@ -72,8 +72,8 @@ def show_aliases(ctx, param, value): @click.group(context_settings=CONTEXT_SETTINGS""" if "invoke_without_command" not in self.manifest.cli_options: self.cli += self.parse_callback_options(callbacks, leading=", ") - if self.manifest.cli_options: - self.cli += f",{self.click_parser.to_args(self.manifest.cli_options)}" + if cli_options := self.cli_options_with_examples(): + self.cli += f",{self.click_parser.to_args(cli_options)}" self.cli += f""") @click.version_option({self.manifest.version!r})""" diff --git a/cliffy/commanders/typer.py b/cliffy/commanders/typer.py index fb00f11..fc1e41a 100644 --- a/cliffy/commanders/typer.py +++ b/cliffy/commanders/typer.py @@ -22,8 +22,10 @@ def add_base_cli(self) -> None: self.cli += """ CONTEXT_SETTINGS = dict(help_option_names=['-h', '--help']) cli = typer.Typer(context_settings=CONTEXT_SETTINGS""" - if self.manifest.cli_options: - self.cli += f",{self.parser.to_args(self.manifest.cli_options)}" + # Typer shows help text as rich markup unless the manifest chooses another mode + rich_markup = self.manifest.cli_options.get("rich_markup_mode", "rich") == "rich" + if cli_options := self.cli_options_with_examples(escape_markup=rich_markup): + self.cli += f",{self.parser.to_args(cli_options)}" if self.manifest.help: self.cli += f", help={py_str(self.manifest.help)}" self.cli += f""") diff --git a/examples/click_hello.yaml b/examples/click_hello.yaml index 507bc78..d8a56dd 100644 --- a/examples/click_hello.yaml +++ b/examples/click_hello.yaml @@ -6,4 +6,11 @@ help: Hello World with Click! commands: hello | hl: $ echo "world" +examples: + - command: clickhello hello + description: Print "world". + - clickhello hl +tests: + - hello: assert result.exit_code == 0 + - --help: assert "Examples:" in result.output and "$ clickhello hl" in result.output diff --git a/examples/generated/clickhello.py b/examples/generated/clickhello.py index 760c3b3..15a7640 100644 --- a/examples/generated/clickhello.py +++ b/examples/generated/clickhello.py @@ -14,7 +14,7 @@ def show_aliases(ctx, param, value): print("\nCommand Aliases\n-------- --------\nhello hl\n") ctx.exit() -@click.group(context_settings=CONTEXT_SETTINGS) +@click.group(context_settings=CONTEXT_SETTINGS, epilog="Examples:\n\n$ clickhello hello # Print \"world\".\n\n$ clickhello hl") @click.version_option('0.1.0') @click.option('--aliases', is_flag=True, callback=show_aliases, expose_value=False, is_eager=True, help='Show command aliases.') diff --git a/examples/generated/hello.py b/examples/generated/hello.py index 2e4cb4f..5f52c5c 100644 --- a/examples/generated/hello.py +++ b/examples/generated/hello.py @@ -4,7 +4,7 @@ import typer CONTEXT_SETTINGS = dict(help_option_names=['-h', '--help']) -cli = typer.Typer(context_settings=CONTEXT_SETTINGS, help="Hello world!") +cli = typer.Typer(context_settings=CONTEXT_SETTINGS, epilog="Examples:\n\n$ hello shell # Say hello from a shell command.\n\n$ hello python # Say hello from Python.", help="Hello world!") __version__ = "0.1.0" __cli_name__ = "hello" diff --git a/examples/hello.yaml b/examples/hello.yaml index c381403..4ad250e 100644 --- a/examples/hello.yaml +++ b/examples/hello.yaml @@ -8,8 +8,15 @@ commands: shell: $echo "hello from shell" python: print("hello from python") +examples: + - command: hello shell + description: Say hello from a shell command. + - command: hello python + description: Say hello from Python. + tests: - shell: assert result.exit_code == 0 - python: assert result.output == "hello from python\n" - shell --help: assert "Show this message and exit" in result.output - - -h: assert "Hello world!" in result.output \ No newline at end of file + - -h: assert "Hello world!" in result.output + - -h: assert "Examples:" in result.output and "$ hello shell" in result.output diff --git a/tests/test_codegen.py b/tests/test_codegen.py index ef621d0..827bf51 100644 --- a/tests/test_codegen.py +++ b/tests/test_codegen.py @@ -8,6 +8,7 @@ import textwrap from importlib import metadata +import click import pytest from click.testing import CliRunner as ClickCliRunner from typer.testing import CliRunner as TyperCliRunner @@ -558,3 +559,114 @@ def test_generated_code_is_reproducible(generate, base): assert first == second assert first.splitlines()[0] == f"## Generated t with cliffy {metadata.version('cliffy')}" assert not re.search(r"\d{4}-\d{2}-\d{2}", first) + + +EXAMPLES = """\ +examples: + - command: t greet "Ada Lovelace" --shout + description: Greet someone, loudly. + - t list --tag [home] --pattern '*.txt' + - command: 't say "it''s \\\\n" ''''''x''''''' + description: | + Quotes and backslashes + are shown as written. +commands: + greet: print(1) + sub.show: print(2) +""" +# as `help_text` returns them: with single spaces +EXAMPLE_LINES = [ + '$ t greet "Ada Lovelace" --shout # Greet someone, loudly.', + "$ t list --tag [home] --pattern '*.txt'", + "$ t say \"it's \\\\n\" '''x''' # Quotes and backslashes are shown as written.", +] + + +def help_text(result) -> str: + """Help output as plain text on one line per paragraph: styles and line wrapping differ between renderers""" + return " ".join(click.unstyle(result.output).split()) + + +@pytest.mark.parametrize("base", [BASE, CLICK], ids=["typer", "click"]) +def test_examples_are_shown_in_the_help_of_the_cli(run, base): + # SOUL principle 7: "examples in the help". They were only used by `cli docs`. + result = run(base + EXAMPLES, ["--help"]) + + assert result.exit_code == 0, result.output + text = help_text(result) + assert "Examples:" in text + for line in EXAMPLE_LINES: + assert line in text + # after the commands, in the order of the manifest + positions = [text.index(marker) for marker in ("greet", "Examples:", *EXAMPLE_LINES)] + assert positions == sorted(positions) + + +@pytest.mark.parametrize("base", [BASE, CLICK], ids=["typer", "click"]) +def test_examples_are_not_repeated_in_the_help_of_commands(run, base): + for args in (["greet", "--help"], ["sub", "--help"], ["sub", "show", "--help"]): + result = run(base + EXAMPLES, args) + assert result.exit_code == 0, result.output + assert "Examples:" not in help_text(result) + + +@pytest.mark.parametrize("base", [BASE, CLICK], ids=["typer", "click"]) +def test_help_has_no_examples_section_without_examples(generate, run, base): + manifest = base + "commands:\n greet: print(1)\n" + + assert "Examples" not in help_text(run(manifest, ["--help"])) + assert "epilog" not in generate(manifest).cli.code + assert "epilog" not in generate(manifest + "examples: []\n").cli.code + + +@pytest.mark.parametrize("base", [BASE, CLICK], ids=["typer", "click"]) +def test_examples_follow_an_epilog_from_cli_options(run, base): + manifest = base + "cli_options:\n epilog: Made with cliffy.\n" + EXAMPLES + + text = help_text(run(manifest, ["--help"])) + + assert text.index("Made with cliffy.") < text.index("Examples:") < text.index(EXAMPLE_LINES[0]) + + +@pytest.mark.parametrize("base", [BASE, CLICK], ids=["typer", "click"]) +@pytest.mark.parametrize( + "example", + [ + '\'"+__import__("os").environ.__setitem__("CLIFFY_PWNED","1")+"\'', + '\'"""+__import__("os").environ.__setitem__("CLIFFY_PWNED","1")+"""\'', + '"\\")\\n__import__(\\"os\\").environ.__setitem__(\\"CLIFFY_PWNED\\",\\"1\\")\\n(\\""', + ], + ids=["quote", "triple_quote", "newline"], +) +@pytest.mark.parametrize("field", ["command", "description", "string"]) +def test_example_text_is_not_executed(run, monkeypatch, base, example, field): + monkeypatch.delenv("CLIFFY_PWNED", raising=False) + if field == "string": + examples = f"examples:\n - {example}\n" + elif field == "command": + examples = f"examples:\n - command: {example}\n" + else: + examples = f"examples:\n - command: t a\n description: {example}\n" + + for args in (["a"], ["--help"]): + result = run(base + examples + "commands:\n a: print(1)\n", args) + assert result.exit_code == 0, result.output + + assert "CLIFFY_PWNED" not in os.environ + assert "CLIFFY_PWNED" in help_text(result) + + +@pytest.mark.parametrize("base", [BASE, CLICK], ids=["typer", "click"]) +def test_example_is_generated_as_one_paragraph_with_its_description(generate, base): + code = generate(base + EXAMPLES).cli.code + + assert r"$ t greet \"Ada Lovelace\" --shout # Greet someone, loudly.\n\n$ t list" in code + + +@pytest.mark.parametrize("mode", ["null", "rich"]) +def test_typer_example_brackets_are_shown_whatever_the_markup_mode(run, mode): + # rich markup reads `[home]` as a tag and drops it. Without markup an escaped `\[home]` would show the backslash. + result = run(BASE + f"cli_options:\n rich_markup_mode: {mode}\n" + EXAMPLES, ["--help"]) + + assert result.exit_code == 0, result.output + assert EXAMPLE_LINES[1] in help_text(result) From 88a4b9af5c2e93576975f81af6896b860d7cdbad Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 17:02:57 -0400 Subject: [PATCH 08/12] Answer invalid JSON-RPC messages in the MCP example instead of exiting A message that is valid JSON but not an object (an array, a string, a number) raised AttributeError in _handle_message and ended the server. It is answered with -32600 Invalid Request, as is a batch array (MCP has no batches since 2025-06-18) and an object without a method. params or arguments that are not objects get -32602. A response object is ignored. The server keeps serving. Co-Authored-By: Claude Opus 5.5 --- examples/blueprints/mcp_tools.py | 36 +++++++++++++++++++++++++----- tests/test_blueprint.py | 38 ++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/examples/blueprints/mcp_tools.py b/examples/blueprints/mcp_tools.py index f106a20..a623827 100644 --- a/examples/blueprints/mcp_tools.py +++ b/examples/blueprints/mcp_tools.py @@ -48,11 +48,30 @@ def _call_tool(name, arguments): return _render_result(TOOLS[name]["function"](**arguments)) +def _error(message_id, code, text): + return {"jsonrpc": "2.0", "id": message_id, "error": {"code": code, "message": text}} + + def _handle_message(message): """Answer one JSON-RPC message. Returns None for notifications, which get no reply.""" - method, params, message_id = message.get("method"), message.get("params") or {}, message.get("id") + if isinstance(message, list): + # MCP has no batches since revision 2025-06-18: one message per line + return _error(None, -32600, "Invalid Request: batch messages are not supported, send one message per line") + if not isinstance(message, dict): + return _error(None, -32600, "Invalid Request: a message must be a JSON object") + + method, params, message_id = message.get("method"), message.get("params"), message.get("id") + if not isinstance(method, str): + # a response to a request is not answered: this server never sends one + if "result" in message or "error" in message: + return None + return _error(message_id, -32600, "Invalid Request: `method` must be a string") if message_id is None: return None + if params is None: + params = {} + if not isinstance(params, dict): + return _error(message_id, -32602, "Invalid params: `params` must be an object") if method == "initialize": result = { @@ -65,13 +84,18 @@ def _handle_message(message): elif method == "tools/list": result = {"tools": _describe_tools()} elif method == "tools/call": + arguments = params.get("arguments") + if arguments is None: + arguments = {} + if not isinstance(arguments, dict): + return _error(message_id, -32602, "Invalid params: `arguments` must be an object") try: - text, failed = _call_tool(params.get("name"), params.get("arguments") or {}), False + text, failed = _call_tool(params.get("name"), arguments), False except Exception as e: text, failed = f"{type(e).__name__}: {e}", True result = {"content": [{"type": "text", "text": text}], "isError": failed} else: - return {"jsonrpc": "2.0", "id": message_id, "error": {"code": -32601, "message": f"Method not found: {method}"}} + return _error(message_id, -32601, f"Method not found: {method}") return {"jsonrpc": "2.0", "id": message_id, "result": result} @@ -80,9 +104,11 @@ def _serve(): if not line.strip(): continue try: - reply = _handle_message(json.loads(line)) + message = json.loads(line) except ValueError: - reply = {"jsonrpc": "2.0", "id": None, "error": {"code": -32700, "message": "Parse error"}} + reply = _error(None, -32700, "Parse error") + else: + reply = _handle_message(message) if reply is not None: sys.stdout.write(json.dumps(reply) + "\\n") sys.stdout.flush() diff --git a/tests/test_blueprint.py b/tests/test_blueprint.py index 1a44dd9..0330cf4 100644 --- a/tests/test_blueprint.py +++ b/tests/test_blueprint.py @@ -503,3 +503,41 @@ def test_example_mcp_blueprint_rejects_bad_specs(): required_flag = {**tool, "parameters": [{"name": "flag", "type": "boolean", "required": True}]} with pytest.raises(BlueprintError, match="a boolean parameter is a flag and cannot be required"): build_manifest(blueprint_cls, {"name": "x", "tools": {"ok": required_flag}}) + + +def test_example_mcp_server_keeps_serving_after_invalid_messages(): + # valid JSON that is not an object raised AttributeError in the server, which then exited + lines = [ + "[]", + '[{"jsonrpc": "2.0", "id": 1, "method": "ping"}]', + '"text"', + "42", + "null", + '{"id": 2}', + '{"jsonrpc": "2.0", "id": 3, "method": "tools/list", "params": [1]}', + '{"jsonrpc": "2.0", "id": 4, "method": "tools/call", "params": {"name": "repeat", "arguments": "ab"}}', + "{not json", + # no reply: a notification, and a response to a request the server never made + '{"jsonrpc": "2.0", "method": "notifications/initialized"}', + '{"jsonrpc": "2.0", "id": 5, "result": {}}', + '{"jsonrpc": "2.0", "id": 6, "method": "ping"}', + ] + + result = CliRunner().invoke(cli, ["run", str(EXAMPLES / "text-tools.yaml"), "--", "serve"], input="\n".join(lines)) + + assert result.exit_code == 0, result.output + replies = [json.loads(line) for line in result.stdout.splitlines()] + assert [(reply["id"], reply.get("error", {}).get("code")) for reply in replies] == [ + (None, -32600), + (None, -32600), + (None, -32600), + (None, -32600), + (None, -32600), + (2, -32600), + (3, -32602), + (4, -32602), + (None, -32700), + (6, None), + ] + assert "batch" in replies[1]["error"]["message"] + assert replies[-1] == {"jsonrpc": "2.0", "id": 6, "result": {}} From 848230208e3ea3556771320413c7dd58870be3f7 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 17:05:25 -0400 Subject: [PATCH 09/12] Never hand a requires entry to pip as an option cli build does not check requires against the installed packages, so an entry such as --index-url=... or -r file reached pip's argument list as an option and changed where it installs from. A manifest or include with such an entry is rejected when it is read, build_cli rejects it for a loaded CLI, and the requirements follow -- in the pip call. Every subprocess call in cliffy/ is an argument list without a shell. Co-Authored-By: Claude Opus 5.5 --- cliffy/builder.py | 10 +++++-- cliffy/helper.py | 16 +++++++++++ cliffy/manifest.py | 16 ++++++++++- tests/test_builder.py | 65 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 103 insertions(+), 4 deletions(-) diff --git a/cliffy/builder.py b/cliffy/builder.py index b4b9994..dd1847e 100644 --- a/cliffy/builder.py +++ b/cliffy/builder.py @@ -24,7 +24,7 @@ from shiv import cli as shiv_cli from cliffy.commander import CLI, SHELL_RUNTIME_IMPORT -from cliffy.helper import import_generated_code, make_executable +from cliffy.helper import import_generated_code, make_executable, option_like_requirement from cliffy.transformer import Transformer DEFAULT_TARGET = "zipapp" @@ -115,7 +115,10 @@ def build(self, job: BuildJob) -> BuildResult: with TemporaryDirectory() as staging: pip_install = [sys.executable, "-m", "pip", "--disable-pip-version-check", "install", "--no-compile"] - pip = subprocess.run([*pip_install, "--target", staging, *job.requirements], capture_output=True, text=True) + # `--` ends the options of pip: whatever follows is a requirement, also if it starts with `-` + pip = subprocess.run( + [*pip_install, "--target", staging, "--", *job.requirements], capture_output=True, text=True + ) if pip.returncode != 0: return self.failed(f"could not install {', '.join(job.requirements)}:\n{last_lines(pip)}") @@ -348,6 +351,9 @@ def build_cli( """ if target not in TARGETS: return BuildResult(1, f"unknown build target `{target}`: choose one of {', '.join(TARGETS)}", target=target) + if problem := option_like_requirement(deps or []): + # a manifest is checked when it is read. The requirements of a loaded CLI come from its metadata record + return BuildResult(1, f"`requires` of {cli_name}: {problem}", target=target) try: code = Path(script_path).read_text(encoding="utf-8") except OSError as e: diff --git a/cliffy/helper.py b/cliffy/helper.py index 2d65f73..6ef9a6b 100644 --- a/cliffy/helper.py +++ b/cliffy/helper.py @@ -241,6 +241,22 @@ def get_installed_package_versions() -> dict[str, str]: return installed_packages +def option_like_requirement(requirements: list[str]) -> str | None: + """Why `requirements` cannot be handed to an installer, or None when they can. + + `cli build` passes the manifest's `requires` to pip. An entry that starts with `-` would be read + as an option of pip (`--index-url ...`, `-r file`) and change where and what it installs from, + so `requires` holds requirements and nothing else. + """ + for requirement in requirements: + if str(requirement).strip().startswith("-"): + return ( + f"`{str(requirement).strip()}` is an option, not a requirement. `requires` lists packages, " + "such as `rich` or `tabulate>=0.9.0`" + ) + return None + + def check_requirement(requirement: str, installed_packages: dict[str, str]) -> str | None: """Checks a PEP 508 requirement against the installed packages. diff --git a/cliffy/manifest.py b/cliffy/manifest.py index adb9f46..3feec87 100644 --- a/cliffy/manifest.py +++ b/cliffy/manifest.py @@ -8,7 +8,7 @@ from pydantic import BaseModel, Field, RootModel, ValidationInfo, field_validator, model_validator -from .helper import CLI_NAME_PATTERN, GENERATED_NAMES, wrap_as_comment +from .helper import CLI_NAME_PATTERN, GENERATED_NAMES, option_like_requirement, wrap_as_comment LATEST_SCHEMA_VERSION = "v3" MIGRATION_GUIDE = "https://jaykv.github.io/cliffy/project/migration/" @@ -373,6 +373,13 @@ class CLIManifest(BaseModel): default=[], description="Example command usages to display in generated CLI docs." ) + @field_validator("requires", mode="after") + @classmethod + def requires_are_requirements(cls, value: list[str]) -> list[str]: + if problem := option_like_requirement(value): + raise ValueError(problem) + return value + @field_validator("name", mode="after") @classmethod def is_safe_name(cls, value: str) -> str: @@ -644,6 +651,13 @@ class IncludeManifest(BaseModel): cli_options: dict[str, str] = {} tests: list[str | dict[str, str]] = [] + @field_validator("requires", mode="after") + @classmethod + def requires_are_requirements(cls, value: list[str]) -> list[str]: + if problem := option_like_requirement(value): + raise ValueError(problem) + return value + class CLIMetadata(BaseModel): """Metadata model""" diff --git a/tests/test_builder.py b/tests/test_builder.py index dab5a6c..2656b6c 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -52,7 +52,9 @@ def pip_requirements(mock_pip_run: MagicMock) -> list[str]: pip_args = mock_pip_run.call_args.args[0] assert pip_args[:3] == [sys.executable, "-m", "pip"] assert "install" in pip_args - return pip_args[pip_args.index("--target") + 2 :] + # the requirements follow `--`, which ends the options of pip + assert pip_args[pip_args.index("--target") + 2] == "--" + return pip_args[pip_args.index("--") + 1 :] def run_artifact( @@ -406,3 +408,64 @@ def test_build_creates_nested_output_dir(tmp_path): assert result.exit_code == 0, result.output assert (output_dir / "script").exists() + + +OPTION_LIKE = ["--index-url=https://packages.invalid/simple", "-r/etc/hosts", " -e .", "--no-deps"] + + +@pytest.mark.parametrize("requirement", OPTION_LIKE) +def test_requirement_that_is_an_option_is_rejected_by_the_manifest(tmp_path, requirement): + # `cli build` does not check `requires` against the installed packages, so the entry went to pip + # as it was: an option, which changed where pip installs from + manifest = tmp_path / "optioned.yaml" + manifest.write_text(f"name: optioned\nversion: '1'\nrequires: [{requirement!r}, rich]\ncommands:\n a: print(1)\n") + + with patch("cliffy.builder.subprocess.run") as run: + result = CliRunner().invoke(cli, ["build", str(manifest), "-o", str(tmp_path / "out")]) + + assert result.exit_code == 1 + run.assert_not_called() + assert f"`{requirement.strip()}` is an option, not a requirement" in result.output + assert "requires" in result.output + assert not (tmp_path / "out").exists() + + +@pytest.mark.parametrize("requirement", OPTION_LIKE) +def test_requirement_that_is_an_option_is_rejected_in_an_include(tmp_path, requirement): + (tmp_path / "shared.yaml").write_text(f"requires: [{requirement!r}]\n") + manifest = tmp_path / "optioned.yaml" + manifest.write_text("name: optioned\nversion: '1'\nincludes: [shared.yaml]\ncommands:\n a: print(1)\n") + + result = CliRunner().invoke(cli, ["render", str(manifest)]) + + assert result.exit_code == 1 + assert f"`{requirement.strip()}` is an option, not a requirement" in result.output + + +@pytest.mark.parametrize("requirement", OPTION_LIKE) +def test_build_does_not_pass_an_option_to_pip(tmp_path, requirement): + # the requirements of a loaded CLI come from its metadata record, not from a validated manifest + script = tmp_path / "demo.py" + script.write_text(TYPER_CODE) + + with patch("cliffy.builder.subprocess.run") as run: + result = build_cli( + "demo", script_path=str(script), deps=["rich", requirement], output_dir=str(tmp_path / "out") + ) + + run.assert_not_called() + assert result.exit_code == 1 + assert f"`{requirement.strip()}` is an option, not a requirement" in result.output + + +def test_requirements_follow_the_end_of_pip_options(tmp_path): + script = tmp_path / "demo.py" + script.write_text(TYPER_CODE) + + with skip_pip() as run: + build_cli("demo", script_path=str(script), deps=["rich>=13"], output_dir=str(tmp_path / "out")) + + command = run.call_args.args[0] + assert command[command.index("--") + 1] == "rich>=13" + assert all(not argument.startswith("-") for argument in command[command.index("--") + 1 :]) + assert "shell" not in run.call_args.kwargs From 6b784c4d6abb3ad0ce33019a3c0e2a998925061d Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 17:06:07 -0400 Subject: [PATCH 10/12] Make the PyPI upload resumable and check that py.typed is shipped uv publish --check-url skips files that are on PyPI already with the same content, so a release whose upload stopped half way can be finished by re-running the job. The build job and a test check that the wheel and the sdist carry cliffy/py.typed and the Typing :: Typed classifier, which they do. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/publish.yaml | 10 +++++++++- tests/test_packaging.py | 10 ++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/test_packaging.py diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index c9f19eb..7e4d555 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -29,6 +29,10 @@ jobs: echo "::error::release tag '$GITHUB_REF_NAME' does not match pyproject version '$version'" exit 1 fi + - name: Check that the distributions ship the type hints marker + run: | + unzip -l dist/*.whl | grep -q ' cliffy/py\.typed$' + tar -tzf dist/*.tar.gz | grep -q '/cliffy/py\.typed$' - name: Smoke test the wheel run: uv run --isolated --no-project --with dist/*.whl cli --version - uses: actions/upload-artifact@v7 @@ -53,5 +57,9 @@ jobs: path: dist/ - name: Install uv uses: astral-sh/setup-uv@v10.3.0 + # --check-url: a file that is on PyPI already, with the same content, is skipped. An upload that + # stopped half way (the sdist is up, the wheel is not) can then be finished with "Re-run failed + # jobs". Without it the files that made it are rejected as duplicates and the retry fails. + # A file with the same name and different content still fails: a release is never replaced. - name: Publish - run: uv publish --trusted-publishing always dist/* + run: uv publish --trusted-publishing always --check-url https://pypi.org/simple/ dist/* diff --git a/tests/test_packaging.py b/tests/test_packaging.py new file mode 100644 index 0000000..a8393b5 --- /dev/null +++ b/tests/test_packaging.py @@ -0,0 +1,10 @@ +"""What the installed package has to carry besides its modules.""" + +from importlib import metadata +from importlib.resources import files + + +def test_package_is_marked_as_typed(): + # without py.typed a type checker ignores the annotations of an installed cliffy (PEP 561) + assert files("cliffy").joinpath("py.typed").is_file() + assert "Typing :: Typed" in (metadata.metadata("cliffy").get_all("Classifier") or []) From de8df79603a8a00873058dc17e2e87b1bbb94a7a Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 17:06:27 -0400 Subject: [PATCH 11/12] Say in SOUL principle 1 which inputs a manifest's CLI depends on A manifest can read environment variables and include files, and its header names the cliffy version, so 'the same manifest produces the same CLI on every machine, every time' was not true as written. The promise is now: the same manifest with the same includes, blueprint and environment variables, read by the same cliffy, gives the same CLI, and cliffy adds no input of its own. Co-Authored-By: Claude Opus 5.5 --- SOUL.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/SOUL.md b/SOUL.md index f3f8545..1e7c28e 100644 --- a/SOUL.md +++ b/SOUL.md @@ -39,7 +39,11 @@ write and a developer can review. That overlap is the project. Everything about a tool lives in its manifest: commands, parameters, help, dependencies, examples and tests. There is no second place to look and no hidden state that changes what a manifest means. -The same manifest produces the same CLI on every machine, every time. +A manifest names its inputs itself: the files it lists under `includes`, its blueprint, +and the environment variables it reads with `{{ env[...] }}`. +The same manifest with the same inputs, read by the same version of cliffy, produces the same CLI +on every machine, every time. +Cliffy adds nothing of its own: not the time, not the machine, not the order something happened to be read in. ### 2. Nothing is hidden From dc9a631982f4c78dd8155c034b1f1683b78ea1c4 Mon Sep 17 00:00:00 2001 From: jaykv Date: Mon, 12 Oct 2026 17:08:24 -0400 Subject: [PATCH 12/12] Document the review fixes and the examples section of --help Changelog and known issues for what changed on this branch; the guides for cli test output and --traceback, examples in --help, cli render to a file, launcher ownership and removal. The examples field description, the JSON schema and the generated reference pages follow. Co-Authored-By: Claude Opus 5.5 --- cliffy/manifest.py | 4 +++- docs/guides/dev-loop.md | 9 +++++++++ docs/guides/generating-docs.md | 15 ++++++++++++++- docs/guides/manifest-basics.md | 5 ++++- docs/guides/testing.md | 24 +++++++++++++++++++++++- docs/project/changelog.md | 31 +++++++++++++++++++++++++++++++ docs/project/known-issues.md | 27 +++++++++++++++++++++++++-- docs/quickstart.md | 3 ++- docs/reference/cli.md | 1 + docs/reference/examples.md | 2 +- docs/reference/manifest.md | 7 ++++--- examples/cliffy_schema.json | 2 +- scripts/docs.py | 5 +++-- 13 files changed, 121 insertions(+), 14 deletions(-) diff --git a/cliffy/manifest.py b/cliffy/manifest.py index 3feec87..5dc5ec1 100644 --- a/cliffy/manifest.py +++ b/cliffy/manifest.py @@ -370,7 +370,9 @@ class CLIManifest(BaseModel): tests: list[str | dict[str, str]] = Field(default=[], description="Test cases for commands") examples: list[str | Example] = Field( - default=[], description="Example command usages to display in generated CLI docs." + default=[], + description="Example command lines of the CLI, with an optional description each. " + "Shown at the end of the `--help` of the generated CLI and in the pages written by `cli docs`.", ) @field_validator("requires", mode="after") diff --git a/docs/guides/dev-loop.md b/docs/guides/dev-loop.md index a47e934..fec3a28 100644 --- a/docs/guides/dev-loop.md +++ b/docs/guides/dev-loop.md @@ -71,5 +71,14 @@ $ cli remove-all # uninstall everything cliffy loaded `cli ls`, `cli reload`, `cli rm` and `cli rm-all` are aliases for these. Details are in the [CLI reference](../reference/cli.md). +Loading writes a small launcher script named after the CLI into the `bin` directory of the +environment. cliffy only replaces or deletes that file while it is exactly the launcher it wrote. If +a file with the name of your CLI is already there (`pip`, say), `cli load` stops and asks you to +rename the CLI. If you edited the launcher, `cli remove` removes nothing, tells you which file is in +the way and leaves the CLI loaded and working; move or delete that file and remove the CLI again. + +While `cli dev` runs, reloads happen one at a time and each one reads the manifest when it starts, +so after a burst of saves the loaded CLI is the one of the last save. + Loaded CLIs belong to the Python environment cliffy is installed in. They are available whenever that environment is active and disappear with it. diff --git a/docs/guides/generating-docs.md b/docs/guides/generating-docs.md index 17b1962..07d016c 100644 --- a/docs/guides/generating-docs.md +++ b/docs/guides/generating-docs.md @@ -5,7 +5,20 @@ with its params and aliases, and the examples you provide. ## Add examples -`examples` is a list of command lines with a description each. It is only used for documentation. +`examples` is a list of command lines with a description each. They are part of the documentation +`cli docs` writes, and the generated CLI shows them at the end of its own `--help`: + +```text + Examples: + + $ notes add "buy milk" --tag home # Add a note tagged home. + + $ notes ls --tag home # Show only the notes tagged home. +``` + +Write an example as you would type it, starting with the name of the CLI. It is shown as written: +quotes, brackets and backslashes are not interpreted. The help of a single command does not repeat +the examples. ```yaml --8<-- "docs/snippets/notes.yaml:examples" diff --git a/docs/guides/manifest-basics.md b/docs/guides/manifest-basics.md index 989c628..cc4ad6d 100644 --- a/docs/guides/manifest-basics.md +++ b/docs/guides/manifest-basics.md @@ -39,7 +39,7 @@ hello world | `cli_options` | Options for the CLI application object. | [below](#cli-options) | | `use_click` | Generate Click code instead of Typer. | [Click and Typer backends](backends.md) | | `tests` | Commands to run and assertions to check. | [Testing manifests](testing.md) | -| `examples` | Example command lines for generated docs. | [Generating docs](generating-docs.md) | +| `examples` | Example command lines, shown in `--help` and in generated docs. | [Generating docs](generating-docs.md) | Keys that cliffy does not know are ignored. The [manifest reference](../reference/manifest.md) lists every key with its type and default. @@ -64,6 +64,9 @@ To read the generated module for any manifest: $ cli render hello.yaml ``` +The output is the module itself, so `cli render hello.yaml > hello.py` writes a file you can run. The +closing `# Rendered ...` line goes to the error stream and does not end up in that file. + ## Imports and functions ```yaml diff --git a/docs/guides/testing.md b/docs/guides/testing.md index edd7f36..1f97986 100644 --- a/docs/guides/testing.md +++ b/docs/guides/testing.md @@ -70,6 +70,27 @@ A failing assertion is reported with its line: 💔 AssertionError: (line 1) > assert result.output == "1\n" ``` +So is an assertion that raises something else, or that is not valid Python: + +```text +💔 NameError: name 'reslt' is not defined (line 1) > assert reslt.exit_code == 0 +``` + +When the command itself raises, the exception is reported against the manifest: the command, the +exception, and the line of the `run`, `pre_run` or `post_run` block it was raised on. + +```text +💥 command `add` raised KeyError: 'notes' + run, line 2: data["notes"].append(text) +``` + +No Python traceback is printed. The assertions still run, so a test can expect the exception with +`assert isinstance(result.exception, KeyError)`. To see the full traceback of the command as well: + +```console +$ cli test testing.yaml --traceback +``` + The run ends with one of three messages: | Last line | Meaning | Exit code | @@ -101,7 +122,8 @@ step of its own. - **The counter counts test cases.** `✅ 2 of 5` is the second test case of five. Shell steps are run but not counted, and a failed case keeps its number. - **Shell steps are not checked.** A step that exits with an error does not fail the run. Assert on - the state it should have produced in the next test case. + the state it should have produced in the next test case. A step whose program cannot be started at + all (it does not exist) ends the run with an error. ## The complete example diff --git a/docs/project/changelog.md b/docs/project/changelog.md index 323d53d..4c1feea 100644 --- a/docs/project/changelog.md +++ b/docs/project/changelog.md @@ -69,6 +69,21 @@ dependency versions. Read the [migration notes](migration.md#from-06x-to-070) be removed with `cli rm`. - `make generate-clis` renders `examples/generated/` with `scripts/examples.py` and no longer installs packages. The `generate-cleanup` target is gone. +- **`cli render` prints exactly the generated module.** Long lines are no longer broken, so + `cli render x.yaml > x.py` is the code that runs. The `# Rendered ...` line goes to stderr. The same + holds for `cli init --render`. +- **`cli test` reports errors against the manifest instead of printing tracebacks.** A command that + raises is shown as `command raised : ` with the line of its run block; an + assertion that raises or is not valid Python is shown with its line; a manifest that does not import + and a shell step that cannot be started end the run with an error message. +- **`cli remove` removes a CLI completely or not at all.** When the launcher is not the file cliffy + wrote, nothing is removed, the CLI stays loaded and the command exits with 1. `cli remove-all` exits + with 1 when a CLI could not be removed. +- **`cli load` only replaces a launcher it wrote itself**: a regular file with exactly the launcher + text for that CLI. A file that merely contained `from cliffy.clis.` used to count as cliffy's own. +- A `requires` entry that starts with `-` is rejected. `requires` lists packages. +- `cli dev` runs one reload at a time. A second save within a second of the first is no longer + dropped. ### Added @@ -84,11 +99,20 @@ dependency versions. Read the [migration notes](migration.md#from-06x-to-070) be CLI or of a group, for both backends. - `vars` values can be numbers and booleans, and a var can be built from other vars in any order. - `cli dev` also watches included manifests and the Python file of a file blueprint. +- The manifest's `examples` are shown at the end of the `--help` of the generated CLI, for both + backends. See [Generating docs](../guides/generating-docs.md#add-examples). +- `cli test --traceback` prints the full Python traceback of a command that raises. ### Security - Text from a manifest (help, names, defaults, versions) is escaped in the generated code and names are validated, so manifest data cannot change the code that is generated. +- The name given to `cli remove`, `update`, `info`, `build` and `docs` is validated. A name such as + `../x` could select and delete files outside of cliffy's own directories. +- Launchers and generated modules are written to a temporary file and renamed into place, a new + launcher is only created when nothing has its name, and symbolic links are never followed. +- `cli build` never passes a `requires` entry to pip as an option. An entry such as + `--index-url=...` used to change where pip installs from. ### Removed @@ -108,6 +132,13 @@ dependency versions. Read the [migration notes](migration.md#from-06x-to-070) be - A Jinja error in a manifest is reported with file, line and the text of the line instead of a traceback. - The field descriptions in the JSON schema and the `cli init` template were wrong in places. +- `cli dev`: a reload that started earlier can no longer finish last and load an older version of the + manifest over a newer one. +- `cli dev --run-cli` and `cli test` on several threads no longer delete each other's temporary files. + Each run has a temporary directory of its own. +- The MCP example blueprint answers a JSON-RPC message that is not an object, or a batch, with an + error instead of exiting. +- Releases: the upload to PyPI can be re-run after it stopped half way. - cliffy type-checks and its test suite passes without the optional extras. The test suite no longer extracts zipapps into `~/.shiv`. - Compatibility with Click 8.2 and later, rich-click 1.9 and Python 3.12 and later (deprecated APIs diff --git a/docs/project/known-issues.md b/docs/project/known-issues.md index 7835c7a..219b563 100644 --- a/docs/project/known-issues.md +++ b/docs/project/known-issues.md @@ -18,8 +18,31 @@ problem against the command it comes from. ### `cli test` does not check shell steps -A `$` or `>` step that exits with an error does not fail the run, and a step whose program does not -exist ends the run with a traceback. See [Testing manifests](../guides/testing.md#things-to-know). +A `$` or `>` step that exits with an error does not fail the run. A step whose program does not +exist ends the run with an error message. See [Testing manifests](../guides/testing.md#things-to-know). + +### Examples are shown for the CLI, not per command + +`examples` belongs to the manifest. The generated CLI shows all of them at the end of its own +`--help`; the help of a single command has no examples section, because a command has no `examples` +key. With the Typer backend an example that contains an emoji code such as `:rocket:` is shown as +the emoji, like the rest of the help text. + +### `cli remove` refuses when the launcher was changed + +The launcher of a loaded CLI is only deleted while it is exactly the file cliffy wrote. After an +edit, or when another program replaced it, `cli remove` and `cli remove-all` remove nothing for that +CLI and exit with 1. + +**Do this instead:** move or delete the file named in the message, then run `cli remove` again. + +### A file can still appear between cliffy's check and its write + +cliffy checks a launcher right before it replaces or deletes it, never follows a symbolic link and +creates a new launcher only when nothing has the name. Replacing or deleting an existing launcher +is a check followed by a rename or an unlink: a file that another process puts at exactly that path +in between is replaced or removed. This needs write access to the environment's `bin` directory at +that moment. ### `cli init` without `--raw` rejects names with a hyphen diff --git a/docs/quickstart.md b/docs/quickstart.md index 358fac5..7292731 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -223,7 +223,8 @@ not need cliffy. With `--target script` you get a single Python script instead, --8<-- "docs/snippets/notes.yaml" ``` -The `examples` section at the end is used by [`cli docs`](guides/generating-docs.md). +The `examples` section at the end is shown at the end of `notes --help` and is used by +[`cli docs`](guides/generating-docs.md). ## Next steps diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 49ba08d..014604f 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -450,6 +450,7 @@ cli test [OPTIONS] MANIFEST |---|---|---|---|---| | `MANIFEST` | file | yes | | Manifest file whose `tests` section should run. | | `-x`, `--exitfirst` | flag | no | | exit instantly on first error or failed test. | +| `--traceback` | flag | no | | Also print the full Python traceback of a command that raises. | | `-b`, `--blueprint` | text | no | | Blueprint that maps a custom YAML spec onto a CLI: a .py file, module:Class or an installed blueprint name. Overrides the `blueprint:` key of the file. | Runs the `tests` section of a manifest and prints each command, its output and the result. The exit diff --git a/docs/reference/examples.md b/docs/reference/examples.md index 6f9dc1f..3516510 100644 --- a/docs/reference/examples.md +++ b/docs/reference/examples.md @@ -30,7 +30,7 @@ described in [Blueprints](../blueprints.md#examples). | [todo.yaml](#todo) | Typer | `tabulate`, `rich` | no | | [requires.yaml](#requires) | Typer | `requests >= 2.30.0`, `six < 1.0.0` | no | | [callbacks.yaml](#callbacks) | Typer | nothing | yes | -| [click_hello.yaml](#clickhello) | Click | nothing | no | +| [click_hello.yaml](#clickhello) | Click | nothing | yes | | [aiprompt.yaml](#aiprompt) | Typer | `openai>=1.0.0`, `rich>=13.0.0`, `keyring>=24.0.0` | no | ## hello diff --git a/docs/reference/manifest.md b/docs/reference/manifest.md index 4025ae8..2353356 100644 --- a/docs/reference/manifest.md +++ b/docs/reference/manifest.md @@ -33,7 +33,7 @@ The top level of a manifest file. | `callbacks` | list of [Callback](#callback) | `[]` | Code that runs before any command of the CLI, or of one command group. Callbacks for the same target run in the order given and their params are combined. The built-in --version and --aliases options are kept. | | `cli_options` | mapping | `{}` | Additional CLI configuration options | | `tests` | list of (string or mapping of name to string) | `[]` | Test cases for commands | -| `examples` | list of (string or [Example](#example)) | `[]` | Example command usages to display in generated CLI docs. | +| `examples` | list of (string or [Example](#example)) | `[]` | Example command lines of the CLI, with an optional description each. Shown at the end of the `--help` of the generated CLI and in the pages written by `cli docs`. | - `name` and `version` are the only required keys. `version` must be a string, so quote values such as `"1.0"`. `commands` can be left out for a CLI that only has `callbacks`. @@ -181,8 +181,9 @@ an existing command group. See [Callbacks](../blueprints.md#callbacks). | `command` | string | **required** | The command line to show. | | `description` | string or null | `null` | A sentence describing what the command does. | -`examples` entries are only used by [`cli docs`](../guides/generating-docs.md). An entry can also be a -plain string, which is shown without a description. +`examples` entries are shown at the end of the `--help` of the generated CLI, as +`$ # `, and in the pages written by [`cli docs`](../guides/generating-docs.md). +An entry can also be a plain string, which is shown without a description. ## Include files diff --git a/examples/cliffy_schema.json b/examples/cliffy_schema.json index 34e3572..9678390 100644 --- a/examples/cliffy_schema.json +++ b/examples/cliffy_schema.json @@ -549,7 +549,7 @@ }, "examples": { "default": [], - "description": "Example command usages to display in generated CLI docs.", + "description": "Example command lines of the CLI, with an optional description each. Shown at the end of the `--help` of the generated CLI and in the pages written by `cli docs`.", "items": { "anyOf": [ { diff --git a/scripts/docs.py b/scripts/docs.py index 0f1a317..b0f82eb 100644 --- a/scripts/docs.py +++ b/scripts/docs.py @@ -530,8 +530,9 @@ def render_cli_reference() -> str: Click. With the Click backend `rich_help_panel` needs rich-click. """, "Example": """ -`examples` entries are only used by [`cli docs`](../guides/generating-docs.md). An entry can also be a -plain string, which is shown without a description. +`examples` entries are shown at the end of the `--help` of the generated CLI, as +`$ # `, and in the pages written by [`cli docs`](../guides/generating-docs.md). +An entry can also be a plain string, which is shown without a description. """, "IncludeManifest": """ Files listed under `includes` are validated against this smaller model. A relative path is looked up