diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c9cc1a6..bdf40e10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Added - Added `skills` to `cloudsmith upstream`. Use `cloudsmith upstream skills list`, `create`, `update`, and `delete` to manage AI Skills upstreams. An AI Skills upstream caches skills and plugins from a GitHub repository. +- Added `--changelog` to `cloudsmith push skills`. Use it to add release notes to an AI Skills package. The option is available only for the `skills` format. +- Added `--compress` to `cloudsmith push skills`. With this option, you can pass a skill folder instead of a `.zip` archive. The CLI compresses the contents of the folder into a `.zip` archive, then uploads it. If you pass a folder without `--compress`, the command fails with a usage error. The option is available only for the `skills` format. ### Changed diff --git a/cloudsmith_cli/cli/commands/push.py b/cloudsmith_cli/cli/commands/push.py index 3cd83111..27df03b8 100644 --- a/cloudsmith_cli/cli/commands/push.py +++ b/cloudsmith_cli/cli/commands/push.py @@ -6,6 +6,8 @@ import math import os import shlex +import shutil +import tempfile import time from datetime import UTC, datetime @@ -1056,6 +1058,34 @@ def upload_files_and_create_package( return slug_perm, slug +def _push_package_file( + ctx, opts, package_file, args, kwargs, metadata_kwargs, metadata, failure_info +): + """Upload one package file, returning ``(result, metadata_info)`` or None.""" + result = None + click.echo(err=utils.should_use_stderr(opts)) + try: + res = upload_files_and_create_package( + ctx, + *args, + **kwargs, + **metadata_kwargs, + package_file=package_file, + metadata=metadata, + metadata_failure_info=failure_info, + ) + if res: + # ``upload_files_and_create_package`` resets and then populates + # ``opts.push_metadata_info`` on every call, so reading it here + # always reflects this file. + result = (res, opts.push_metadata_info) + except ApiException: + utils.echo_skipped_error(opts) + + click.echo(err=utils.should_use_stderr(opts)) + return result + + def _implied_dsc_file(package_file): """Return ``package_file`` if it is itself a Debian ``.dsc``, else None.""" if isinstance(package_file, str) and package_file.endswith(DSC_SUFFIX): @@ -1136,7 +1166,7 @@ def create_push_handlers(): "package_file", nargs=1 if has_additional_params else -1, type=ExpandPath( - dir_okay=False, exists=True, writable=False, resolve_path=True + dir_okay=key == "skills", exists=True, writable=False, resolve_path=True ), ) @click.option( @@ -1277,6 +1307,10 @@ def push_handler(ctx, *args, **kwargs): if cli_failure_mode is not None: opts.cli_metadata_failure_mode = cli_failure_mode + # ``--compress`` is a client-side switch, not a + # package-create field, so keep it out of the API payload. + compress = kwargs.pop("compress", False) + package_files = kwargs.pop("package_file") if not isinstance(package_files, tuple): package_files = (package_files,) @@ -1301,29 +1335,53 @@ def push_handler(ctx, *args, **kwargs): **metadata_kwargs, opts=opts ) + def push_file(package_file): + result = _push_package_file( + ctx, + opts, + package_file, + args, + kwargs, + metadata_kwargs, + metadata, + metadata_failure_info, + ) + if result: + results.append(result) + results = [] for package_file in package_files: - kwargs["package_file"] = package_file - - try: - click.echo(err=utils.should_use_stderr(opts)) - res = upload_files_and_create_package( - ctx, - *args, - **kwargs, - **metadata_kwargs, - metadata=metadata, - metadata_failure_info=metadata_failure_info, + # Case: If a directory is selected as package upload target (only for AI skills at the moment) + # but does not specify the `--compress flag` + if os.path.isdir(package_file) and not compress: + raise click.BadParameter( + "Directories require --compress.", + param_hint="PACKAGE_FILE", ) - if res: - # ``upload_files_and_create_package`` resets and then - # populates ``opts.push_metadata_info`` on every call, - # so reading it here always reflects this iteration. - results.append((res, opts.push_metadata_info)) - except ApiException: - utils.echo_skipped_error(opts) + # Case: If a directory is selected as package upload target and flag is set -> zip the directory + elif os.path.isdir(package_file) and compress: + # Upload inside the temp dir context so the archive still exists + with tempfile.TemporaryDirectory() as tmp: + use_stderr = utils.should_use_stderr(opts) + + click.echo( + f"Compressing {click.style(package_file, bold=True)} directory ... ", + nl=False, + err=use_stderr, + ) - click.echo(err=utils.should_use_stderr(opts)) + archive = shutil.make_archive( + base_name=os.path.join(tmp, os.path.basename(package_file)), + format="zip", + root_dir=package_file, + ) + + click.secho("OK", fg="green", nl=False, err=use_stderr) + + push_file(archive) + # Default case: regular package file + else: + push_file(package_file) if utils.should_use_stderr(opts): data = [] @@ -1412,6 +1470,22 @@ def push_handler(ctx, *args, **kwargs): ), )(push_handler) + elif key == "skills": + push_handler = click.option( + "--changelog", + "changelog", + default=None, + help="Changelog for skills", + )(push_handler) + + push_handler = click.option( + "--compress", + "compress", + default=False, + is_flag=True, + help="Compress a skills directory into a .zip before upload", + )(push_handler) + handlers[key] = push_handler diff --git a/cloudsmith_cli/cli/tests/test_push_skills_cli.py b/cloudsmith_cli/cli/tests/test_push_skills_cli.py new file mode 100644 index 00000000..1ca1e689 --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_push_skills_cli.py @@ -0,0 +1,236 @@ +"""Registered-command tests for ``cloudsmith push skills``.""" + +import os +import zipfile +from unittest.mock import patch + +import pytest + +from .. import config as cli_config +from ..commands.main import main + +_MOCK_TARGETS = ( + "cloudsmith_cli.cli.commands.push.validate_create_package", + "cloudsmith_cli.cli.commands.push.validate_upload_file", + "cloudsmith_cli.cli.commands.push.upload_file", + "cloudsmith_cli.cli.commands.push.create_package", + "cloudsmith_cli.cli.commands.push.wait_for_package_sync", +) + + +@pytest.fixture(autouse=True) +def hermetic_environment(monkeypatch): + """Keep developer environment and config values out of command tests.""" + monkeypatch.delenv("CLOUDSMITH_ORG", raising=False) + monkeypatch.delenv("CLOUDSMITH_CONFIG_FILE", raising=False) + monkeypatch.delattr(cli_config.OPTIONS, "value", raising=False) + monkeypatch.setattr(cli_config.ConfigReader, "config_files", ["config.ini"]) + monkeypatch.setattr(cli_config.ConfigReader, "config_searchpath", ["."]) + + +def test_changelog_is_sent_to_the_api(runner, tmp_path): + package_file = tmp_path / "my-skill.zip" + package_file.write_bytes(b"dummy content") + + with ( + patch(_MOCK_TARGETS[0]) as mock_validate_create_package, + patch(_MOCK_TARGETS[1], return_value="checksum"), + patch(_MOCK_TARGETS[2], return_value="uploaded:my-skill.zip"), + patch( + _MOCK_TARGETS[3], return_value=("slug-perm", "slug") + ) as mock_create_package, + patch(_MOCK_TARGETS[4]), + ): + result = runner.invoke( + main, + [ + "push", + "skills", + "example/repo", + str(package_file), + "--name", + "my-skill", + "--version", + "1.0.0", + "--changelog", + "Fixed a bug", + "--api-key", + "fake-api-key", + ], + catch_exceptions=False, + ) + + assert result.exit_code == 0, result.output + + # Validation runs before file upload -> check kwargs passed through + validation_kwargs = mock_validate_create_package.call_args.kwargs + assert validation_kwargs["changelog"] == "Fixed a bug" + + # The create call persists the package -> check kwargs gets sent to package creation + create_kwargs = mock_create_package.call_args.kwargs + assert create_kwargs["changelog"] == "Fixed a bug" + + +def test_push_skills_help_documents_changelog_option(runner): + result = runner.invoke(main, ["push", "skills", "--help"], catch_exceptions=False) + + assert result.exit_code == 0, result.output + assert "--changelog" in result.output + + +def test_push_non_skills_format_has_no_changelog_option(runner): + result = runner.invoke(main, ["push", "raw", "--help"], catch_exceptions=False) + + assert result.exit_code == 0, result.output + assert "--changelog" not in result.output + + +def test_directory_is_compressed_and_uploaded_with_compress(runner, tmp_path): + skill_dir = tmp_path / "my-skill" + (skill_dir / "scripts").mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("---\nname: my-skill\n---\n") + (skill_dir / "scripts" / "run.sh").write_text("echo hi\n") + + uploaded = {} + + # The archive lives in a temp dir that is removed after the push, so + # inspect it while the upload is in flight. + def capture_upload(*args, filepath, **kwargs): + with zipfile.ZipFile(filepath) as archive: + uploaded["filename"] = os.path.basename(filepath) + uploaded["members"] = sorted(archive.namelist()) + return "uploaded:my-skill.zip" + + with ( + patch(_MOCK_TARGETS[0]) as mock_validate_create_package, + patch(_MOCK_TARGETS[1], return_value="checksum"), + patch(_MOCK_TARGETS[2], side_effect=capture_upload), + patch( + _MOCK_TARGETS[3], return_value=("slug-perm", "slug") + ) as mock_create_package, + patch(_MOCK_TARGETS[4]), + ): + result = runner.invoke( + main, + [ + "push", + "skills", + "example/repo", + str(skill_dir), + "--name", + "my-skill", + "--version", + "1.0.0", + "--compress", + "--api-key", + "fake-api-key", + ], + catch_exceptions=False, + ) + + assert result.exit_code == 0, result.output + assert uploaded["filename"] == "my-skill.zip" + # Members are relative to the skill directory, not the absolute path + assert "SKILL.md" in uploaded["members"] + assert "scripts/run.sh" in uploaded["members"] + + # The flag is client-side only and must not leak into the API payload + assert "compress" not in mock_validate_create_package.call_args.kwargs + assert "compress" not in mock_create_package.call_args.kwargs + + +def test_directory_without_compress_is_rejected(runner, tmp_path): + skill_dir = tmp_path / "my-skill" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("---\nname: my-skill\n---\n") + + with ( + patch(_MOCK_TARGETS[0]), + patch(_MOCK_TARGETS[1]), + patch(_MOCK_TARGETS[2]) as mock_upload_file, + patch(_MOCK_TARGETS[3]) as mock_create_package, + patch(_MOCK_TARGETS[4]), + ): + result = runner.invoke( + main, + [ + "push", + "skills", + "example/repo", + str(skill_dir), + "--name", + "my-skill", + "--version", + "1.0.0", + "--api-key", + "fake-api-key", + ], + ) + + assert result.exit_code == 2, result.output + assert "Directories require --compress" in result.output + assert not mock_upload_file.called + assert not mock_create_package.called + + +def test_file_with_compress_is_uploaded_unchanged(runner, tmp_path): + package_file = tmp_path / "my-skill.zip" + package_file.write_bytes(b"dummy content") + + with ( + patch(_MOCK_TARGETS[0]), + patch(_MOCK_TARGETS[1], return_value="checksum"), + patch( + _MOCK_TARGETS[2], return_value="uploaded:my-skill.zip" + ) as mock_upload_file, + patch(_MOCK_TARGETS[3], return_value=("slug-perm", "slug")), + patch(_MOCK_TARGETS[4]), + ): + result = runner.invoke( + main, + [ + "push", + "skills", + "example/repo", + str(package_file), + "--name", + "my-skill", + "--version", + "1.0.0", + "--compress", + "--api-key", + "fake-api-key", + ], + catch_exceptions=False, + ) + + assert result.exit_code == 0, result.output + assert mock_upload_file.call_args.kwargs["filepath"] == str(package_file) + + +def test_push_skills_help_documents_compress_option(runner): + result = runner.invoke(main, ["push", "skills", "--help"], catch_exceptions=False) + + assert result.exit_code == 0, result.output + assert "--compress" in result.output + + +def test_push_non_skills_format_rejects_directory(runner, tmp_path): + package_dir = tmp_path / "not-a-file" + package_dir.mkdir() + + result = runner.invoke( + main, + [ + "push", + "raw", + "example/repo", + str(package_dir), + "--api-key", + "fake-api-key", + ], + ) + + assert result.exit_code == 2, result.output + assert "is a directory" in result.output + assert "--compress" not in result.output