diff --git a/CHANGELOG.md b/CHANGELOG.md index f7d9966a..af1117a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,50 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Added + +- Auto-conversion of "bare-FQN" plugins: a plugin whose manifest `kind` is a Python class + path (e.g. `package.module.ClassName`) instead of a known kind is now converted to an + `isolated_venv` plugin at install time (the FQN is moved into `default_config.class_name`), + so it runs out-of-process in a per-plugin virtual environment ([#113](https://github.com/contextforge-org/cpex/pull/113)). +- `--no-convert` flag on `cpex plugin install` to opt out of the conversion above and keep + the plugin's declared FQN `kind` (loaded in-process). `--no-convert` also softens an + unknown/unsupported `kind` from a hard error to a warning. Applies to pypi/test-pypi/git/local + installs ([#113](https://github.com/contextforge-org/cpex/pull/113)). + +### Changed + +- **Runtime model of existing FQN-declared Python plugins.** On 0.1.x, declaring a plugin + `kind` as a Python class path was how in-process Python plugins were declared. Because + conversion is now **on by default**, upgrading changes such plugins from in-process to the + out-of-process `isolated_venv` model unless installed with `--no-convert`. Conversion also + runs during `cpex plugin catalog update` and persists the converted form to + `plugin-manifest.yaml` / `plugins/config.yaml` ([#113](https://github.com/contextforge-org/cpex/pull/113)). + +### Fixed + +- **Isolated worker: error responses could carry a stale `request_id`.** The worker reused a + `main()`-local `request_id` across loop iterations, so an error raised before the next task + parsed could be tagged with the previous request's id — misdelivering the error to the wrong + caller's queue or hanging the real caller until timeout. The id is now reset per iteration + ([#113](https://github.com/contextforge-org/cpex/pull/113)). +- **`cpex plugin install pkg@` could wrongly skip.** The repeat-install check + dropped the version constraint and, for pypi/test-pypi, compared against a possibly stale + catalog entry. An explicit version constraint now always proceeds with the install + ([#113](https://github.com/contextforge-org/cpex/pull/113)). +- **Upgrade no longer force-rebuilds every existing `isolated_venv` venv.** The venv cache now + treats a *missing* manifest version/hash signal (metadata written by an earlier CLI) as "no + signal" rather than a mismatch, so pre-existing venvs are not wiped and rebuilt on the first + run after upgrade ([#113](https://github.com/contextforge-org/cpex/pull/113)). +- **Multi-plugin packages no longer thrash the venv cache.** The persisted plugin manifest is + now keyed on the plugin's full class name instead of the shared package root, so installing + one plugin in a package no longer invalidates a sibling plugin's cache hash and triggers a + rebuild loop ([#113](https://github.com/contextforge-org/cpex/pull/113)). +- **test-pypi isolated installs resolve transitive dependencies.** Installing a plugin from + test.pypi into a fresh isolated venv now also passes `--extra-index-url https://pypi.org/simple/`, + so transitive dependencies (including `cpex` itself) resolve from real PyPI instead of failing + when they are absent from test.pypi ([#113](https://github.com/contextforge-org/cpex/pull/113)). + ## [0.1.1] - 2026-06-04 ### Added diff --git a/cpex/framework/constants.py b/cpex/framework/constants.py index 64d8e090..7ba64555 100644 --- a/cpex/framework/constants.py +++ b/cpex/framework/constants.py @@ -12,8 +12,26 @@ # Model constants. # Specialized plugin types. +NATIVE_PLUGIN_TYPE = "native" +BUILTIN_PLUGIN_TYPE = "builtin" +WASM_PLUGIN_TYPE = "wasm" EXTERNAL_PLUGIN_TYPE = "external" ISOLATED_VENV_PLUGIN_TYPE = "isolated_venv" +PDP_PLUGIN_TYPE = "PDP" + +# The set of manifest `kind` values the installer recognizes as-is. Any `kind` +# outside this set is a candidate for FQN auto-conversion to isolated_venv +# (see cpex.tools.catalog.classify_plugin_kind). +KNOWN_PLUGIN_KINDS = frozenset( + { + BUILTIN_PLUGIN_TYPE, + NATIVE_PLUGIN_TYPE, + WASM_PLUGIN_TYPE, + EXTERNAL_PLUGIN_TYPE, + ISOLATED_VENV_PLUGIN_TYPE, + PDP_PLUGIN_TYPE, + } +) # MCP related constants. diff --git a/cpex/framework/isolated/client.py b/cpex/framework/isolated/client.py index c61a3981..9e66b4aa 100644 --- a/cpex/framework/isolated/client.py +++ b/cpex/framework/isolated/client.py @@ -27,7 +27,7 @@ from cpex.framework.hooks.registry import get_hook_registry from cpex.framework.isolated.venv_comm import VenvProcessCommunicator from cpex.framework.models import PluginConfig, PluginContext, PluginErrorModel, PluginPayload, PluginResult -from cpex.framework.utils import find_package_path +from cpex.framework.utils import find_package_path, manifest_filename_for_class logger = logging.getLogger(__name__) @@ -51,27 +51,61 @@ def __init__(self, config: PluginConfig, plugin_dirs) -> None: self.cache_dir: Path = cache_root / ".cpex" / "venv_cache" self.cache_dir.mkdir(parents=True, exist_ok=True) - def _compute_requirements_hash(self, requirements_file: str) -> str: + def _compute_requirements_hash(self, requirements_file: Optional[str]) -> str: """Compute SHA256 hash of requirements file content. Args: - requirements_file: Path to the requirements file + requirements_file: Path to the requirements file, or None when the + plugin has no requirements file. Returns: - Hexadecimal hash string + Hexadecimal hash string. An absent or non-existent file hashes to + the empty-content digest. """ hasher = hashlib.sha256() - req_path = Path(requirements_file) - if req_path.exists(): - with open(req_path, "rb") as f: - hasher.update(f.read()) + if requirements_file is not None: + req_path = Path(requirements_file) + if req_path.exists(): + with open(req_path, "rb") as f: + hasher.update(f.read()) + else: + # If no requirements file, use empty hash + hasher.update(b"") else: - # If no requirements file, use empty hash hasher.update(b"") return hasher.hexdigest() + def _manifest_path(self) -> Path: + """Path to the plugin's persisted manifest under its plugin directory. + + The filename is keyed on the full class name so plugins that share a + package (and therefore this directory and its venv) do not collide on a + single manifest file — which would make each install invalidate the + others' cache hash. Must match the write side in + PluginCatalog._persist_manifest_to_plugin_dir. + """ + class_name = self.config.config.get("class_name") + return self.plugin_path / manifest_filename_for_class(class_name) + + def _compute_manifest_hash(self) -> str: + """Compute SHA256 hash of the persisted plugin-manifest.yaml. + + Returns the empty-content digest when no manifest file is present. This + gives plugins without a self-referencing requirements file a stable + change signal for cache invalidation (U5): a version bump or any edit to + the persisted manifest changes the hash and forces a venv reinstall. + """ + hasher = hashlib.sha256() + manifest_path = self._manifest_path() + if manifest_path.exists(): + with open(manifest_path, "rb") as f: + hasher.update(f.read()) + else: + hasher.update(b"") + return hasher.hexdigest() + def _get_cache_metadata_path(self, venv_path: str) -> Path: """Get the path to the cache metadata file. @@ -84,12 +118,14 @@ def _get_cache_metadata_path(self, venv_path: str) -> Path: venv_name = Path(venv_path).name return self.cache_dir / f"{venv_name}_metadata.json" - def _is_venv_cache_valid(self, venv_path: str, requirements_file: str) -> bool: - """Check if cached venv is valid by comparing requirements hash. + def _is_venv_cache_valid(self, venv_path: str, requirements_file: Optional[str]) -> bool: + """Check if cached venv is valid by comparing requirements + manifest signals. Args: venv_path: Path to the virtual environment - requirements_file: Path to the requirements file + requirements_file: Path to the requirements file, or None when the + plugin has no requirements file (manifest version+hash is then + the sole change signal). Returns: True if cache is valid, False otherwise @@ -115,12 +151,40 @@ def _is_venv_cache_valid(self, venv_path: str, requirements_file: str) -> bool: # Compute current requirements hash current_hash = self._compute_requirements_hash(requirements_file) - # Compare hashes + # Compare requirements hash cached_hash = metadata.get("requirements_hash") if cached_hash != current_hash: logger.info("Requirements changed. Cached hash: %s, Current hash: %s", cached_hash, current_hash) return False + # Compare manifest version + hash (U5). Either changing invalidates + # the cache — this is the sole change signal for plugins with no + # requirements file (whose requirements hash is a constant). + # + # A *missing* key means the metadata predates these signals (written + # by an earlier CLI): treat it as "no signal" and skip the check, + # rather than reading .get() -> None as a mismatch. Otherwise every + # existing isolated_venv install would be wiped and rebuilt on the + # first run after upgrade, contradicting plan R5. Only invalidate + # when the key is present and differs. + if "manifest_version" in metadata: + current_version = self.config.version + cached_version = metadata.get("manifest_version") + if cached_version != current_version: + logger.info("Manifest version changed. Cached: %s, Current: %s", cached_version, current_version) + return False + + if "manifest_hash" in metadata: + current_manifest_hash = self._compute_manifest_hash() + cached_manifest_hash = metadata.get("manifest_hash") + if cached_manifest_hash != current_manifest_hash: + logger.info( + "Manifest content changed. Cached hash: %s, Current hash: %s", + cached_manifest_hash, + current_manifest_hash, + ) + return False + logger.info("Valid venv cache found for %s", venv_path) return True @@ -128,20 +192,27 @@ def _is_venv_cache_valid(self, venv_path: str, requirements_file: str) -> bool: logger.warning("Error reading cache metadata: %s", str(e)) return False - def _save_cache_metadata(self, venv_path: str, requirements_file: str) -> None: + def _save_cache_metadata(self, venv_path: str, requirements_file: Optional[str]) -> None: """Save cache metadata for the venv. Args: venv_path: Path to the virtual environment - requirements_file: Path to the requirements file + requirements_file: Path to the requirements file, or None when the + plugin has no requirements file. """ metadata_path = self._get_cache_metadata_path(venv_path) requirements_hash = self._compute_requirements_hash(requirements_file) + resolved_requirements = None + if requirements_file is not None and Path(requirements_file).exists(): + resolved_requirements = str(Path(requirements_file).resolve()) + metadata = { "venv_path": str(Path(venv_path).resolve()), - "requirements_file": str(Path(requirements_file).resolve()) if Path(requirements_file).exists() else None, + "requirements_file": resolved_requirements, "requirements_hash": requirements_hash, + "manifest_version": self.config.version, + "manifest_hash": self._compute_manifest_hash(), "python_version": f"{sys.version_info.major}.{sys.version_info.minor}.{sys.version_info.micro}", } @@ -162,8 +233,10 @@ async def create_venv( """ venv_path_obj = Path(venv_path) - # Check if we can use cached venv - if use_cache and requirements_file and self._is_venv_cache_valid(venv_path, requirements_file): + # Check if we can use cached venv. A None requirements_file is valid: + # for converted FQN plugins the manifest version+hash is the cache signal + # (U5), so cache validity must be evaluated even without a requirements file. + if use_cache and self._is_venv_cache_valid(venv_path, requirements_file): logger.info("✓ Using cached virtual environment at: %s", venv_path_obj.resolve()) return False @@ -209,38 +282,38 @@ async def initialize(self) -> None: venv_path = self.plugin_path / ".venv" - # Prevent directory traversal: ensure requirements_file stays within plugin_path - requirements_file_input = self.config.config["requirements_file"] - - # Handle both relative and absolute paths - if isinstance(requirements_file_input, Path): - requirements_file = requirements_file_input - else: - requirements_file = Path(requirements_file_input) - - # Try to find the package location where plugin-manifest.yaml resides - # Fall back to self.plugin_path if package is not installed (e.g., in tests) - try: - package_path = find_package_path(self.config.name) - logger.debug("Found installed package %s at %s", self.config.name, package_path) - except RuntimeError: - # Package not installed (e.g., in test environment), use plugin_path - package_path = self.plugin_path - logger.debug("Package %s not installed, using plugin_path: %s", self.config.name, package_path) - - requirements_file = package_path / requirements_file_input - - # Create venv with caching support + # requirements_file is optional: converted FQN plugins get their package + # into the venv via the installer's channel source, not a requirements file. + requirements_file_input = self.config.config.get("requirements_file") + + requirements_file: Optional[Path] = None + if requirements_file_input: + # Try to find the package location where plugin-manifest.yaml resides. + # Fall back to self.plugin_path if package is not installed (e.g., in tests). + try: + package_path = find_package_path(self.config.name) + logger.debug("Found installed package %s at %s", self.config.name, package_path) + except RuntimeError: + # Package not installed (e.g., in test environment), use plugin_path + package_path = self.plugin_path + logger.debug("Package %s not installed, using plugin_path: %s", self.config.name, package_path) + + requirements_file = package_path / requirements_file_input + + # Create venv with caching support. create_venv tolerates a None/missing + # requirements file (empty-hash path). new_venv = await self.create_venv(venv_path=venv_path, requirements_file=requirements_file, use_cache=True) self.comm = VenvProcessCommunicator(venv_path) - # Only install requirements if venv was newly created or cache was invalid - # Check if we need to install requirements + # Only (re)install when the venv was newly created or the cache was invalid. if new_venv: - logger.info("Installing requirements in venv") - self.comm.install_requirements(requirements_file) - # Save metadata after successful installation + if requirements_file is not None and Path(requirements_file).exists(): + logger.info("Installing requirements in venv") + self.comm.install_requirements(requirements_file) + else: + logger.info("No requirements file for %s; skipping requirements install", self.config.name) + # Save metadata after successful creation (records requirements hash for cache validity). self._save_cache_metadata(venv_path, requirements_file) else: logger.info("Using cached venv, skipping requirements installation") diff --git a/cpex/framework/isolated/worker.py b/cpex/framework/isolated/worker.py index 74deee05..bb9c7cfb 100644 --- a/cpex/framework/isolated/worker.py +++ b/cpex/framework/isolated/worker.py @@ -36,7 +36,7 @@ class TaskProcessor: config_hash: str module_path_hash: str - hook_ref: HookRef + plugin_ref: PluginRef executor: PluginExecutor plugin_config: PluginConfig | None = None @@ -55,19 +55,26 @@ def compute_hash(self, json_config_or_module_path: str): def initialize( self, - hook_ref: HookRef, + plugin_ref: PluginRef, executor: PluginExecutor, json_config: str, module_path: str, plugin_config: PluginConfig, ): """Assign locals, and compute hashes.""" - self.hook_ref = hook_ref + self.plugin_ref = plugin_ref self.executor = executor self.config_hash = self.compute_hash(json_config_or_module_path=json_config) self.module_path_hash = self.compute_hash(json_config_or_module_path=module_path) self.plugin_config = plugin_config + def get_hook_ref(self, hook_type: str) -> HookRef: + """ + make sure that the hook ref is not stale for the current task data. + """ + hook_ref = HookRef(hook_type, self.plugin_ref) + return hook_ref + def get_environment_info(): """Get information about current Python environment.""" @@ -112,7 +119,6 @@ async def process_task(task_data, tp: TaskProcessor): if tp.config_hash != tp.compute_hash(json_config): # pull the resolved plugin path and only add the module path if it has the same root config: PluginConfig = PluginConfig(**config_raw) - hook_type = task_data.get(HOOK_TYPE) cls_name: str = task_data.get("class_name") mod_name, n_cls_name = parse_class_name(cls_name) module: ModuleType = import_module(mod_name) @@ -121,12 +127,10 @@ async def process_task(task_data, tp: TaskProcessor): plugin_type = cast(Type[Plugin], class_) plugin = plugin_type(config) await plugin.initialize() - # now invoke the hook plugin_ref = PluginRef(plugin) - hook_ref = HookRef(hook_type, plugin_ref) executor = PluginExecutor(None, 30) tp.initialize( - hook_ref=hook_ref, + plugin_ref=plugin_ref, executor=executor, json_config=json_config, module_path=json.dumps(resolved_paths), @@ -134,12 +138,21 @@ async def process_task(task_data, tp: TaskProcessor): ) # retrieve the context context = task_data.get("context") + hook_type = task_data.get(HOOK_TYPE) plugin_context = PluginContext( state=context.get("state"), global_context=context.get("global_context"), metadata=context.get("metadata") ) + # The client serializes the payload with model_dump(mode="json") before + # sending it over stdin, so it arrives here as a plain dict. Reconstruct + # the typed PluginPayload (e.g. ToolPreInvokePayload) before invoking the + # plugin — otherwise the hook receives a dict and attribute access such as + # payload.args raises AttributeError. This mirrors the response path, which + # rebuilds results via json_to_result on the client side. + raw_payload = task_data.get("payload") + payload = tp.plugin_ref.plugin.json_to_payload(hook_type, raw_payload) if raw_payload is not None else None result = await tp.executor.execute_plugin( - hook_ref=tp.hook_ref, - payload=task_data.get("payload"), + hook_ref=tp.get_hook_ref(hook_type), + payload=payload, local_context=plugin_context, violations_as_exceptions=False, ) @@ -151,6 +164,50 @@ async def process_task(task_data, tp: TaskProcessor): } +def read_task_line(max_content_size: int | None) -> tuple[str, bool]: + """Read one task line from stdin, enforcing max_content_size. + + TextIOWrapper.readline takes a positional size hint (readline(size=-1, /)); + there is no `limit` keyword. readline(size) returns *at most* size chars, + stopping early at a newline. So if we read exactly max_content_size chars + with no trailing newline, the task was truncated mid-line and the rest is + still queued on stdin — if left there it would be mis-read as the next task, + desyncing the request_id-demuxed stream. In that case we drain the remainder + (bounded, discarded) so the next read starts on a fresh line. + + A line that is exactly max_content_size chars *including* its newline is a + complete, valid task, not a truncation — hence the endswith check. + + Returns (line, oversized). When oversized is True the line was truncated and + its content should not be parsed; the caller should reject the request. An + empty line signals EOF. + """ + if max_content_size: + line = sys.stdin.readline(int(max_content_size)) + else: + # on the first read, the plugin_config has not yet been initialized so just read. + line = sys.stdin.readline() + + if not (max_content_size and len(line) == max_content_size and not line.endswith("\n")): + return line, False + + # Drain the rest of the oversized line in bounded chunks so we never buffer + # the giant remainder into memory. Track total drained length so the log + # reflects how far over the limit the offending line actually was. + drained_len = len(line) + while True: + remainder = sys.stdin.readline(int(max_content_size)) + drained_len += len(remainder) + if not remainder or remainder.endswith("\n"): + break + logger.error( + "Task line exceeds max content size (max=%d, read at least %d chars); rejecting request", + max_content_size, + drained_len, + ) + return line, True + + async def main(): """Main function - continuously read from stdin, process tasks, write to stdout.""" logger.info("Worker process started, waiting for tasks...") @@ -160,18 +217,34 @@ async def main(): tp = TaskProcessor() # Continuously read and process tasks while True: + # Reset per iteration so an error before the task is parsed never + # emits a *previous* request's id (venv_comm demuxes strictly on + # request_id; a stale id misdelivers the error or hangs the caller). + request_id = "unknown" try: - # Read one line at a time - if tp.plugin_config and "max_content_size" in tp.plugin_config: - line = sys.stdin.readline(limit=int(tp.plugin_config.max_content_size)) - else: - # on the first read, the plugin_config has not yet been initialized so just read. - line = sys.stdin.readline() + # Read one line at a time. getattr rather than `in`/attribute + # access because plugin_config is a PluginConfig model, not a + # dict, and it may be None on the first read (config not yet + # initialized) or lack the field on older cpex versions. + max_content_size = getattr(tp.plugin_config, "max_content_size", None) + line, oversized = read_task_line(max_content_size) # Check for EOF if not line: logger.info("EOF received, shutting down worker") break + # An oversized (truncated) line was already drained from stdin by + # read_task_line; its content is not reliably parseable JSON, so + # request_id is unrecoverable and stays "unknown". Reject it. + if oversized: + error_response = { + "status": "error", + "message": "Task line exceeds max content size", + "request_id": request_id, + } + print(json.dumps(error_response), flush=True) + continue + # Parse the task task_data = json.loads(line.strip()) request_id = task_data.get("request_id", "unknown") @@ -197,18 +270,19 @@ async def main(): serialized_response = json.dumps(serializable_response) # Send response back to parent (one line per response) - if tp.plugin_config: - # workaround until cpex is updated beyond dev11 - # cpex is a dependency of the plugin and as such it's PluginConfig does not contain the max_content_size yet. - if "max_content_size" in tp.plugin_config: - if len(serialized_response) > tp.plugin_config.max_content_size: - logger.error("Serialized response exceeds max content size") - error_response = { - "status": "error", - "message": "Serialized response exceeds max content size", - "request_id": request_id, - } - serialized_response = json.dumps(error_response) + # workaround until cpex is updated beyond dev11: older cpex + # (a dependency of the plugin) has a PluginConfig without + # max_content_size, so use getattr rather than `in`/attribute + # access. PluginConfig is a model, not a dict, so `in` raises. + response_max_content_size = getattr(tp.plugin_config, "max_content_size", None) + if response_max_content_size and len(serialized_response) > response_max_content_size: + logger.error("Serialized response exceeds max content size") + error_response = { + "status": "error", + "message": "Serialized response exceeds max content size", + "request_id": request_id, + } + serialized_response = json.dumps(error_response) print(serialized_response, flush=True) except json.JSONDecodeError as e: @@ -224,7 +298,10 @@ async def main(): error_response = { "status": "error", "message": f"Unexpected error: {str(e)}", - "request_id": "unknown", + # request_id is reset to "unknown" at the top of each loop + # iteration and set once the task line parses, so callers + # can demux without risk of a stale id from a prior request. + "request_id": request_id, } print(json.dumps(error_response), flush=True) diff --git a/cpex/framework/models.py b/cpex/framework/models.py index e85ffcf3..d2346e39 100644 --- a/cpex/framework/models.py +++ b/cpex/framework/models.py @@ -1599,7 +1599,11 @@ class PluginManifest(BaseModel): Attributes: name (str): The name of the plugin. - kind (str): The class name (for native plugins) | external | isolated_venv + kind (str): The class name (for native plugins) | external | isolated_venv. + A bare Python class path (FQN, e.g. ``package.module.ClassName``) is + auto-converted to ``isolated_venv`` at install time (moving the FQN into + ``default_config.class_name``), unless the install is run with ``--no-convert``, + in which case the FQN kind is kept and loaded in-process. description (str): A description of the plugin. author (str): The author of the plugin. version (str): version of the plugin. diff --git a/cpex/framework/utils.py b/cpex/framework/utils.py index 7eb4a94c..cacf9c4c 100644 --- a/cpex/framework/utils.py +++ b/cpex/framework/utils.py @@ -190,6 +190,32 @@ def parse_class_name(name: str) -> tuple[str, str]: return ("", name) +def manifest_filename_for_class(class_name: str) -> str: + """Return the per-plugin manifest filename for a full class name. + + Plugins that share a package share one venv directory (keyed on the class + root), but each needs its own persisted manifest so the venv cache signal + (manifest hash) does not collide between, e.g., ``pkg.a.PluginA`` and + ``pkg.b.PluginB`` — otherwise installing one invalidates the other's hash + and both rebuild in a loop. Keying the filename on the full, sanitized + class name gives each plugin a distinct manifest within the shared dir. + + The write side (catalog persistence) and the read side (venv cache + validation) MUST call this so they resolve the identical path. + + Args: + class_name: The plugin's fully-qualified class path + (e.g. ``pkg.module.ClassName``). + + Returns: + A filesystem-safe manifest filename, e.g. + ``pkg-module-ClassName.plugin-manifest.yaml``. + """ + safe = "".join(c if (c.isalnum() or c in ("-", "_")) else "-" for c in class_name.strip()) + safe = safe.strip("-") or "plugin" + return f"{safe}.plugin-manifest.yaml" + + def normalize_content_type(content_type: str) -> str: """Extract base content type without parameters. diff --git a/cpex/tools/catalog.py b/cpex/tools/catalog.py index 5d689743..0ef1ea1d 100644 --- a/cpex/tools/catalog.py +++ b/cpex/tools/catalog.py @@ -28,6 +28,7 @@ from github import Auth, Github from packaging.version import InvalidVersion, Version +from cpex.framework.constants import ISOLATED_VENV_PLUGIN_TYPE, KNOWN_PLUGIN_KINDS from cpex.framework.models import ( GitRepo, PluginManifest, @@ -36,7 +37,7 @@ PluginVersionRegistry, PyPiRepo, ) -from cpex.framework.utils import find_package_path +from cpex.framework.utils import find_package_path, manifest_filename_for_class from cpex.tools.integrity import ( IntegrityVerificationError, fetch_pypi_package_hashes, @@ -47,6 +48,133 @@ logger = logging.getLogger(__name__) +# Result buckets for classify_plugin_kind. +KIND_KNOWN = "known" +KIND_FQN = "fqn" +KIND_REJECT = "reject" + + +def classify_plugin_kind(kind: str | None) -> str: + """Classify a manifest ``kind`` value for install-time handling. + + Buckets: + - ``KIND_KNOWN``: ``kind`` is one of the recognized plugin kinds and is + handled by the existing install path unchanged. + - ``KIND_FQN``: ``kind`` is not a known kind but looks like a Python + fully-qualified class path (e.g. ``pkg.module.ClassName``). Such a + plugin is auto-converted to ``isolated_venv`` at install time. + - ``KIND_REJECT``: ``kind`` is neither a known kind nor a class-shaped + FQN (e.g. a typo like ``isolate_venv`` or a single bare token). The + caller raises with a message naming the supported kinds. + + Detection is purely shape-based; no import or venv is required. An FQN must + have two or more dot-separated segments, each a valid Python identifier, + with the final segment starting with an uppercase letter (class convention). + + Args: + kind: The manifest ``kind`` value, or None. + + Returns: + One of ``KIND_KNOWN``, ``KIND_FQN``, or ``KIND_REJECT``. + """ + if not kind or not kind.strip(): + return KIND_REJECT + + kind = kind.strip() + if kind in KNOWN_PLUGIN_KINDS: + return KIND_KNOWN + + segments = kind.split(".") + if len(segments) < 2: + return KIND_REJECT + if not all(seg.isidentifier() for seg in segments): + return KIND_REJECT + if not segments[-1][:1].isupper(): + return KIND_REJECT + + return KIND_FQN + + +def supported_kinds_message() -> str: + """Return an error message naming the supported plugin kinds. + + Used when a ``kind`` is rejected by :func:`classify_plugin_kind`. + """ + kinds = ", ".join(sorted(KNOWN_PLUGIN_KINDS)) + return ( + "Unsupported plugin 'kind'. Expected one of: " + f"{kinds}; or a Python class path (e.g. 'package.module.ClassName') " + "to auto-convert to an isolated_venv plugin." + ) + + +def convert_fqn_kind_in_place(manifest_data: dict[str, Any], convert: bool = True) -> dict[str, Any]: + """Auto-convert a bare-FQN plugin manifest to isolated_venv, in place. + + When ``manifest_data["kind"]`` is a Python class path rather than a known + kind (per :func:`classify_plugin_kind`), the FQN is moved into + ``default_config["class_name"]`` and ``kind`` is set to ``isolated_venv``. + An existing ``class_name`` is preserved (the explicit value wins; a mismatch + is logged). Known kinds pass through untouched. A ``kind`` that is neither + known nor a class-shaped FQN raises. + + Assumes legacy ``default_configs`` has already been normalized to + ``default_config`` by the caller. + + Args: + manifest_data: Raw manifest dict, mutated in place. + convert: When True (the default), auto-convert an FQN ``kind`` and raise + on an unsupported (rejected) kind. When False, the conversion is an + opt-out no-op: FQN kinds are left untouched so they load in-process, + and a rejected kind is logged as a warning and left unchanged instead + of raising. This backs the ``--no-convert`` install flag, letting + existing 0.1.x plugins keep their declared FQN ``kind``. + + Returns: + The same ``manifest_data`` dict, for convenience. + + Raises: + ValueError: If ``kind`` is unsupported and not a class-shaped FQN, and + ``convert`` is True. + """ + kind = manifest_data.get("kind") + bucket = classify_plugin_kind(kind) + + if not convert: + # Opt-out: leave the manifest untouched. A rejected kind is only a + # warning here (it may still be a valid in-process kind the caller + # intends to keep) rather than a hard failure. + if bucket == KIND_REJECT: + logger.warning("Leaving kind %r unconverted (--no-convert): %s", kind, supported_kinds_message()) + return manifest_data + + if bucket == KIND_KNOWN: + return manifest_data + if bucket == KIND_REJECT: + raise ValueError(supported_kinds_message()) + + # KIND_FQN: move the class path into default_config.class_name. + default_config = manifest_data.get("default_config") + if not isinstance(default_config, dict): + default_config = {} + manifest_data["default_config"] = default_config + + existing = default_config.get("class_name") + if existing: + if existing != kind: + logger.warning( + "Manifest kind '%s' is an FQN but default_config.class_name '%s' " + "is already set; keeping the explicit class_name.", + kind, + existing, + ) + else: + default_config["class_name"] = kind + + logger.info("Auto-converting FQN plugin kind '%s' to isolated_venv", kind) + manifest_data["kind"] = ISOLATED_VENV_PLUGIN_TYPE + return manifest_data + class PluginCatalog: """ @@ -354,6 +482,20 @@ def _transform_manifest_data( if "default_configs" in manifest_content: manifest_content["default_config"] = manifest_content.pop("default_configs") or {} + # Auto-convert bare-FQN kinds to isolated_venv (U2) so the persisted + # catalog manifest reflects the converted kind for the monorepo path. + # During catalog population we must not drop a manifest over an + # unrecognized kind — leave it unconverted and let the install path + # surface the rejection when the user actually installs it. + try: + convert_fqn_kind_in_place(manifest_content) + except ValueError as e: + logger.warning( + "Leaving kind %r unconverted during catalog scan: %s", + manifest_content.get("kind"), + str(e), + ) + return manifest_content def _process_manifest_item( @@ -646,7 +788,11 @@ def install_folder_via_pip(self, manifest: PluginManifest, verify_integrity: boo repo_url, manifest.name, verify_integrity=verify_integrity ) plugin_path = self._initialize_isolated_venv(manifest, package_path) - logger.info("Isolated venv initialized. Plugin will be auto-installed via requirements.txt") + # Install the plugin package into the isolated venv from the + # monorepo subdirectory source so its class path is importable + # even without a requirements file (U4). + self._install_package_into_venv(plugin_path, [repo_url]) + logger.info("Isolated venv initialized and plugin package installed from monorepo source") else: # For non-isolated plugins, install normally into CLI's venv logger.info("Installing non-isolated plugin from monorepo: %s", manifest.name) @@ -744,7 +890,7 @@ def _load_manifest_file(self, manifest_path: Path) -> dict[str, Any]: raise RuntimeError(f"Error reading manifest file: {str(e)}") from e def _normalize_manifest_data( - self, manifest_data: dict[str, Any], package_name: str, version_constraint: str | None + self, manifest_data: dict[str, Any], package_name: str, version_constraint: str | None, convert: bool = True ) -> PluginManifest: """Transform raw manifest dict into validated PluginManifest model. @@ -752,6 +898,9 @@ def _normalize_manifest_data( manifest_data: Raw manifest dictionary from YAML. package_name: The PyPI package name. version_constraint: Optional version constraint. + convert: When True (default), auto-convert a bare-FQN ``kind`` to + ``isolated_venv``. When False (``--no-convert``), leave the kind + untouched and warn (rather than raise) on an unsupported kind. Returns: Validated PluginManifest instance. @@ -768,6 +917,10 @@ def _normalize_manifest_data( if "default_config" not in manifest_data and "default_configs" in manifest_data: manifest_data["default_config"] = manifest_data.pop("default_configs") or {} + # Auto-convert bare-FQN kinds to isolated_venv (U2). Skipped when + # convert is False (--no-convert), which also softens reject to warn. + convert_fqn_kind_in_place(manifest_data, convert=convert) + # Validate and create manifest manifest = PluginManifest(**manifest_data) @@ -802,6 +955,41 @@ def _persist_manifest(self, manifest: PluginManifest, package_name: str) -> None except Exception as e: raise RuntimeError(f"Failed to save manifest for {package_name}: {str(e)}") from e + def _persist_manifest_to_plugin_dir(self, manifest: PluginManifest) -> Path | None: + """Persist the (converted) manifest under plugins//. + + For isolated_venv plugins this writes plugin-manifest.yaml into the + plugin's own directory (the ``.venv`` sibling), giving a stable on-disk + record that the venv cache keys its manifest hash on (U5, R3). The + directory is derived from the plugin's ``class_name`` root, matching + IsolatedVenvPlugin.plugin_path. + + Args: + manifest: The validated (already converted) plugin manifest. + + Returns: + The path to the written manifest file, or None when the plugin is + not isolated_venv or has no class_name. + """ + if manifest.kind != ISOLATED_VENV_PLUGIN_TYPE: + return None + class_name = manifest.default_config.get("class_name") + if not class_name: + return None + + class_root = class_name.split(".")[0] + plugin_dir = Path(self.plugin_folder) / class_root + plugin_dir.mkdir(parents=True, exist_ok=True) + # Key the manifest filename on the full class name, not the shared + # class_root: multiple plugins in one package share this directory (and + # its venv), so a single "plugin-manifest.yaml" would collide and make + # each install invalidate the others' cache hash. See + # manifest_filename_for_class and IsolatedVenvPlugin._manifest_path. + manifest_path = plugin_dir / manifest_filename_for_class(class_name) + manifest_path.write_text(yaml.safe_dump(manifest.model_dump(), default_flow_style=False), encoding="utf-8") + logger.info("Persisted converted manifest to %s", manifest_path) + return manifest_path + @staticmethod def _safe_zip_extract(zip_ref: zipfile.ZipFile, extract_dir: Path) -> None: """Extract a zip archive, rejecting members whose paths escape extract_dir.""" @@ -1117,10 +1305,28 @@ def _initialize_isolated_venv(self, manifest: PluginManifest, package_path: Path config=plugin_config, plugin_dirs=[str(self.plugin_folder)], ) + # requirements_file is optional. Converted FQN plugins have no + # requirements file — their package is installed into the venv from + # the install channel's source instead (U4). Only copy when the + # manifest declares one and it exists in the package. # TODO: sec - prevent path traversal on user supplied requirements file path. - requirements_file = manifest.default_config.get("requirements_file", "requirements.txt") - source_path = self._find_requirements_in_extracted_package(package_path, manifest.name, requirements_file) - shutil.copy(source_path, isolated_plugin.plugin_path / requirements_file) + requirements_file = manifest.default_config.get("requirements_file") + if requirements_file: + try: + source_path = self._find_requirements_in_extracted_package( + package_path, manifest.name, requirements_file + ) + shutil.copy(source_path, isolated_plugin.plugin_path / requirements_file) + except FileNotFoundError: + logger.info( + "No requirements file '%s' found for %s; continuing without one", + requirements_file, + manifest.name, + ) + # Persist the (converted) manifest into the plugin dir BEFORE venv + # init, so the venv cache metadata can hash it as a change signal (U5). + self._persist_manifest_to_plugin_dir(manifest) + # Initialize the venv (this will create venv and install requirements) import asyncio import concurrent.futures @@ -1251,6 +1457,36 @@ def _get_venv_python_executable(self, venv_path: Path) -> str: return str(python_exe) + def _install_package_into_venv(self, plugin_path: Path, pip_args: list[str]) -> None: + """Install a plugin package into a plugin's isolated venv. + + Used for converted FQN plugins (and any isolated_venv plugin without a + self-referencing requirements file): the plugin's package is installed + into ``plugin_path/.venv`` from the install channel's own source so the + plugin's class path is importable by the worker. When a requirements + file is present it is installed first; this layers on top of it. + + Args: + plugin_path: The plugin directory containing the ``.venv``. + pip_args: Arguments passed after ``pip install`` (e.g. the package + spec, a git URL, or ``["-e", ""]``, plus any index flags). + + Raises: + RuntimeError: If the install subprocess fails. + """ + venv_python = self._get_venv_python_executable(plugin_path / ".venv") + logger.info("Installing plugin package into isolated venv: %s", " ".join(pip_args)) + try: + subprocess.run( + [venv_python, "-m", "pip", "install", *pip_args], + check=True, + capture_output=True, + text=True, + timeout=600, + ) + except subprocess.CalledProcessError as e: + raise RuntimeError(f"Failed to install plugin package into venv: {e.stderr}") from e + def _handle_plugin_installation( self, manifest: PluginManifest, package_path: Path, install_command: list[str] | None = None ) -> Path | None: @@ -1328,6 +1564,7 @@ def install_from_pypi( version_constraint: str | None = None, use_pytest: bool = False, verify_integrity: bool = True, + convert: bool = True, ) -> tuple[PluginManifest, Path | None]: """Install Python package from PyPI and load its plugin-manifest.yaml. @@ -1367,7 +1604,9 @@ def install_from_pypi( manifest_data = self._load_manifest_file(manifest_path) # Step 3: Normalize and validate the manifest - manifest = self._normalize_manifest_data(manifest_data, plugin_package_name, version_constraint) + manifest = self._normalize_manifest_data( + manifest_data, plugin_package_name, version_constraint, convert=convert + ) package_path = manifest_path.parent @@ -1378,8 +1617,33 @@ def install_from_pypi( install_command=None, # Will install separately for non-isolated ) - # For non-isolated plugins, install via pip and find package path - if manifest.kind != "isolated_venv": + if manifest.kind == "isolated_venv": + # Install the plugin package into the isolated venv from PyPI so + # its class path is importable even without a requirements file (U4). + if plugin_path is None: + raise RuntimeError(f"Failed to initialize isolated venv for {manifest.name}") + tgt = plugin_package_name + if version_constraint is not None: + tgt = f"{tgt}{version_constraint}" + # Fresh empty venv: unlike the CLI venv install, no dependencies + # are pre-resolved here, so transitive deps (including cpex, on + # which the whole isolated design rests) must resolve too. Pair + # test.pypi with real PyPI as an extra index so those deps are + # found even when only the plugin itself lives on test.pypi. + pip_args = ( + [ + "--index-url", + "https://test.pypi.org/simple/", + "--extra-index-url", + "https://pypi.org/simple/", + tgt, + ] + if use_pytest + else [tgt] + ) + self._install_package_into_venv(plugin_path, pip_args) + else: + # For non-isolated plugins, install via pip and find package path self._install_package(plugin_package_name, version_constraint, use_pytest) plugin_path = find_package_path(plugin_package_name) @@ -1393,7 +1657,9 @@ def install_from_pypi( if temp_extract_dir.exists(): shutil.rmtree(temp_extract_dir.parent) - def install_from_git(self, url: str, verify_integrity: bool = True) -> tuple[PluginManifest, Path | None]: + def install_from_git( + self, url: str, verify_integrity: bool = True, convert: bool = True + ) -> tuple[PluginManifest, Path | None]: """Install Python package from Git repository and load its plugin-manifest.yaml. This method performs the following steps: @@ -1519,7 +1785,7 @@ def install_from_git(self, url: str, verify_integrity: bool = True) -> tuple[Plu manifest_data = self._load_manifest_file(manifest_path) # Step 4: Normalize and validate the manifest - manifest = self._normalize_manifest_data(manifest_data, package_name, None) + manifest = self._normalize_manifest_data(manifest_data, package_name, None, convert=convert) # Update the manifest with the git repo information git_repo: GitRepo = GitRepo( @@ -1540,18 +1806,10 @@ def install_from_git(self, url: str, verify_integrity: bool = True) -> tuple[Plu # Install the package from git if manifest.kind == "isolated_venv": - # Install into isolated venv + # Install into isolated venv from the git source (U4). if plugin_path is None: raise RuntimeError(f"Failed to initialize isolated venv for {manifest.name}") - venv_python = self._get_venv_python_executable(plugin_path / ".venv") - logger.info("Installing package into isolated venv: %s", install_url) - subprocess.run( - [venv_python, "-m", "pip", "install", install_url], - check=True, - capture_output=True, - text=True, - timeout=600, - ) + self._install_package_into_venv(plugin_path, [install_url]) logger.info("Successfully installed into isolated venv") else: # Install into current venv @@ -1636,7 +1894,7 @@ def uninstall_package(self, package_name: str, manifest: PluginManifest) -> bool except Exception as e: raise RuntimeError(f"Unexpected error uninstalling {package_name}: {str(e)}") from e - def install_from_local(self, source: Path) -> tuple[PluginManifest, Path]: + def install_from_local(self, source: Path, convert: bool = True) -> tuple[PluginManifest, Path]: """Install a plugin from a local source directory. This method performs the following steps: @@ -1716,7 +1974,9 @@ def install_from_local(self, source: Path) -> tuple[PluginManifest, Path]: # Step 2: Load and parse the manifest manifest_data = self._load_manifest_file(manifest_path) - manifest = self._normalize_manifest_data(manifest_data, pyproject_data["project"]["name"], None) + manifest = self._normalize_manifest_data( + manifest_data, pyproject_data["project"]["name"], None, convert=convert + ) manifest.local = str(source.resolve()) logger.info("Loaded manifest for plugin: %s (kind: %s)", manifest.name, manifest.kind) @@ -1745,6 +2005,10 @@ def install_from_local(self, source: Path) -> tuple[PluginManifest, Path]: plugin_dirs=[str(self.plugin_folder)], ) + # Persist the (converted) manifest into the plugin dir BEFORE + # venv init so the cache metadata can hash it (U5). + self._persist_manifest_to_plugin_dir(manifest) + # Initialize the venv (creates venv directory structure) import asyncio import concurrent.futures @@ -1760,19 +2024,9 @@ def install_from_local(self, source: Path) -> tuple[PluginManifest, Path]: with concurrent.futures.ThreadPoolExecutor(max_workers=1) as ex: ex.submit(asyncio.run, isolated_plugin.initialize()).result() - # Get the venv python executable - venv_path = isolated_plugin.plugin_path / ".venv" - venv_python = self._get_venv_python_executable(venv_path) - - # Install the plugin in editable mode into the isolated venv - logger.info("Installing plugin in editable mode into isolated venv: %s", venv_path) - subprocess.run( - [venv_python, "-m", "pip", "install", "-e", str(source)], - check=True, - capture_output=True, - text=True, - timeout=600, - ) + # Install the plugin in editable mode into the isolated venv from + # the local source (U4). + self._install_package_into_venv(isolated_plugin.plugin_path, ["-e", str(source)]) plugin_path = isolated_plugin.plugin_path logger.info("Successfully installed %s into isolated venv at %s", manifest.name, plugin_path) diff --git a/cpex/tools/cli.py b/cpex/tools/cli.py index 07c4c982..d1419c98 100644 --- a/cpex/tools/cli.py +++ b/cpex/tools/cli.py @@ -480,6 +480,79 @@ def _parse_pypi_source(source: str) -> tuple[str, Optional[str]]: return package_name, version_constraint +def _plugin_name_from_source(source: str, install_type: str | None) -> str: + """Extract the plugin/package name from an install source string. + + Handles the per-channel source shapes: pypi ``pkg@constraint``, + git ``pkg @ git+url``, and bare names / monorepo search terms. + + Args: + source: The install source string. + install_type: The install channel type, or None (defaults to monorepo). + + Returns: + The plugin/package name portion of the source. + """ + if install_type in {"pypi", "test-pypi"}: + name, _ = _parse_pypi_source(source) + return name.strip() + if " @ " in source: # git: "Name @ git+url" + return source.split(" @ ", 1)[0].strip() + return source.strip() + + +def _should_skip_reinstall(source: str, install_type: str | None, catalog: PluginCatalog) -> bool: + """Decide whether an install of an already-registered plugin is a no-op (U6). + + When the plugin is not yet installed, returns False (proceed with install). + When it is installed, compares the target version (from the catalog, when + resolvable) against the recorded installed version and skips only when they + match. If the target version cannot be resolved, proceeds with the install + so the downstream venv cache key (U5) decides whether work is actually + needed. Kind-agnostic — applies to every plugin kind. + + Args: + source: The install source string. + install_type: The install channel type. + catalog: The plugin catalog (already updated for monorepo/git). + + Returns: + True to skip the install (already at the requested version), else False. + """ + registry = PluginRegistry() + name = _plugin_name_from_source(source, install_type) + installed = registry.get(name) + if installed is None: + return False + + # An explicit version constraint (e.g. "foo@==0.3.0") is an intentional + # request for a specific version. Never skip in that case: the constraint + # is not carried into the comparison below, and for pypi/test-pypi/local + # the catalog is deliberately not refreshed (see the install command), so + # catalog.find(name) may return a stale, unrelated monorepo entry. Defer to + # the real install (and the downstream venv cache key) instead of guessing. + if install_type in {"pypi", "test-pypi"}: + _, version_constraint = _parse_pypi_source(source) + if version_constraint is not None: + return False + + target_manifest = catalog.find(name) + target_version = target_manifest.version if target_manifest is not None else None + + if target_version is not None and target_version == installed.version: + console.print(f"Plugin {name} is already installed at version {installed.version}.") + return True + + if target_version is None: + # Cannot resolve a target version to compare; proceed and let the venv + # cache key decide whether a rebuild is needed. + logger.info("Could not resolve target version for %s; proceeding with install.", name) + return False + + console.print(f"Plugin {name} is installed at {installed.version}; installing version {target_version}.") + return False + + def _finalize_installation( manifest: PluginManifest, install_type: str, catalog: PluginCatalog, plugin_path: Path | None = None ): @@ -503,12 +576,14 @@ def _finalize_installation( update_plugins_config_yaml(manifest=manifest) -def _install_from_local(source: str, catalog: PluginCatalog, use_test: bool = False): +def _install_from_local(source: str, catalog: PluginCatalog, use_test: bool = False, convert: bool = True): """Handle local-based installation (not yet implemented). Args: source: local path. catalog: The plugin catalog. + use_test: Unused for local installations (kept for handler consistency). + convert: When False (``--no-convert``), leave a bare-FQN ``kind`` unconverted. Raises: FileNotFoundError: If plugin-manifest.yaml is not found in source or subdirectories. @@ -516,18 +591,19 @@ def _install_from_local(source: str, catalog: PluginCatalog, use_test: bool = Fa """ install_source = Path(source) with console.status(f"Installing plugin from source {source}...", spinner="dots"): - manifest, installation_path = catalog.install_from_local(install_source) + manifest, installation_path = catalog.install_from_local(install_source, convert=convert) _finalize_installation(manifest, "local", catalog, installation_path) console.print(f":white_heavy_check_mark: {manifest.name} installation complete.") -def _install_from_git(source: str, catalog: PluginCatalog, use_test: bool = False): +def _install_from_git(source: str, catalog: PluginCatalog, use_test: bool = False, convert: bool = True): """Handle git-based installation. Args: source: Git repository URL or path. catalog: The plugin catalog. use_test: Unused for git installations (kept for consistency). + convert: When False (``--no-convert``), leave a bare-FQN ``kind`` unconverted. """ # Get integrity verification setting from catalog settings catalog_settings = get_catalog_settings() @@ -539,18 +615,25 @@ def _install_from_git(source: str, catalog: PluginCatalog, use_test: bool = Fals console.log("Package integrity verification: disabled") with console.status(f"Installing plugin from source {source}...", spinner="dots"): - manifest, installation_path = catalog.install_from_git(source, verify_integrity=verify_integrity) + manifest, installation_path = catalog.install_from_git( + source, verify_integrity=verify_integrity, convert=convert + ) _finalize_installation(manifest, "git", catalog, installation_path) console.print(f":white_heavy_check_mark: {manifest.name} installation complete.") -def _install_from_monorepo(source: str, catalog: PluginCatalog, use_test: bool = False, assume_yes: bool = False): +def _install_from_monorepo( + source: str, catalog: PluginCatalog, use_test: bool = False, assume_yes: bool = False, convert: bool = True +): """Handle monorepo-based installation. Args: source: Plugin name or search term in the monorepo. catalog: The plugin catalog. assume_yes: Skip the interactive selection prompt. + convert: Accepted for signature parity; monorepo installs use the + already-normalized catalog manifest, so conversion happened at + ``catalog update`` time and ``--no-convert`` does not apply here. """ logger.info("Trying to install from git monorepo: %s", source) available_plugins = catalog.search(source) @@ -569,13 +652,14 @@ def _install_from_monorepo(source: str, catalog: PluginCatalog, use_test: bool = console.print(f":white_heavy_check_mark: {selected_plugin.name} installation complete.") -def _install_from_pypi(source: str, catalog: PluginCatalog, use_test: bool = False): +def _install_from_pypi(source: str, catalog: PluginCatalog, use_test: bool = False, convert: bool = True): """Handle PyPI-based installation. Args: source: PyPI package name, optionally with version constraint (e.g., "package@>=1.0.0"). catalog: The plugin catalog. use_test: Whether to use test.pypi.org instead of pypi.org. + convert: When False (``--no-convert``), leave a bare-FQN ``kind`` unconverted. """ logger.info("Trying to install from pypi package %s", source) @@ -597,6 +681,7 @@ def _install_from_pypi(source: str, catalog: PluginCatalog, use_test: bool = Fal version_constraint=version_constraint, use_pytest=use_test, verify_integrity=verify_integrity, + convert=convert, ) if manifest is None: @@ -607,7 +692,9 @@ def _install_from_pypi(source: str, catalog: PluginCatalog, use_test: bool = Fal console.print(f":white_heavy_check_mark: {package_name} installation complete.") -def install(source: str, install_type: str | None, catalog: PluginCatalog, assume_yes: bool = False): +def install( + source: str, install_type: str | None, catalog: PluginCatalog, assume_yes: bool = False, convert: bool = True +): """Install a plugin from its associated source. Args: @@ -615,6 +702,10 @@ def install(source: str, install_type: str | None, catalog: PluginCatalog, assum install_type: The type of installation ("git", "monorepo", or "pypi"). catalog: The catalog of plugins. assume_yes: Skip interactive selection prompt for monorepo installs. + convert: When False (``--no-convert``), leave a bare-FQN ``kind`` unconverted + so the plugin keeps its declared in-process class path instead of being + auto-converted to ``isolated_venv``. Applies to pypi/test-pypi/git/local; + monorepo installs use the pre-normalized catalog manifest. Raises: typer.Exit: With EXIT_INVALID_ARGS if install_type is not supported. @@ -625,7 +716,7 @@ def install(source: str, install_type: str | None, catalog: PluginCatalog, assum if install_type == "monorepo": try: - _install_from_monorepo(source, catalog, assume_yes=assume_yes) + _install_from_monorepo(source, catalog, assume_yes=assume_yes, convert=convert) return except Exception as e: console.print(f":x: Installation failed: {str(e)}") @@ -647,7 +738,7 @@ def install(source: str, install_type: str | None, catalog: PluginCatalog, assum raise typer.Exit(EXIT_INVALID_ARGS) try: - handler(source, catalog, use_test=True if install_type == "test-pypi" else False) + handler(source, catalog, use_test=True if install_type == "test-pypi" else False, convert=convert) except Exception as e: console.print(f":x: Installation failed: {str(e)}") logger.error("Install error: %s", str(e), exc_info=True) @@ -850,6 +941,17 @@ def plugin( help="Output format for read commands: 'text' (default) or 'json'.", ), ] = "text", + no_convert: Annotated[ + bool, + typer.Option( + "--no-convert", + help=( + "On install, do NOT auto-convert a bare Python class path (FQN) 'kind' to an " + "isolated_venv plugin; keep the plugin's declared in-process kind. Also softens " + "an unknown 'kind' from a hard error to a warning. Applies to pypi/test-pypi/git/local." + ), + ), + ] = False, ) -> None: """Lists installed plugins""" if cmd_action == "info": @@ -862,12 +964,6 @@ def plugin( raise typer.Exit(EXIT_INVALID_ARGS) pc = PluginCatalog() return uninstall(source, catalog=pc, assume_yes=assume_yes) - if cmd_action == "install" and source is not None: - registry = PluginRegistry() - if registry.has(source): - console.print(f"Plugin {source} is already installed.") - return - # update the catalog before proceeding with install etc. pc = PluginCatalog() # optimized github search REST api takes ~14s to search & download all manifests @@ -880,13 +976,21 @@ def plugin( else: console.log("Catalog update completed.") + # Repeat-install version compare (U6): when a plugin is already registered, + # only skip when the requested/catalog version matches the installed version; + # otherwise fall through and reinstall (upgrade). This is kind-agnostic and + # applies to all plugin kinds. + if cmd_action == "install" and source is not None: + if _should_skip_reinstall(source, install_type, pc): + return + if cmd_action == "versions": return versions(source, catalog=pc, fmt=fmt) if cmd_action == "list": return list_registered_plugins(install_type, fmt=fmt) if cmd_action == "install" and source is not None: - return install(source, install_type, catalog=pc, assume_yes=assume_yes) + return install(source, install_type, catalog=pc, assume_yes=assume_yes, convert=not no_convert) if cmd_action == "search": return search(source, catalog=pc, fmt=fmt) diff --git a/cpex/tools/plugin_registry.py b/cpex/tools/plugin_registry.py index 29f4dda8..385d1d12 100644 --- a/cpex/tools/plugin_registry.py +++ b/cpex/tools/plugin_registry.py @@ -125,6 +125,20 @@ def has(self, plugin_name: str) -> bool: return True return False + def get(self, plugin_name: str) -> InstalledPluginInfo | None: + """Return the installed plugin record by name, or None if not installed. + + Args: + plugin_name: The name of the plugin to look up. + + Returns: + The InstalledPluginInfo for the plugin, or None. + """ + for plugin in self.registry.plugins: + if plugin.name == plugin_name: + return plugin + return None + def remove(self, plugin_name: str) -> bool: """ Remove a plugin from the registry. diff --git a/docs/plans/2026-07-13-001-feat-fqn-plugin-auto-conversion-plan.md b/docs/plans/2026-07-13-001-feat-fqn-plugin-auto-conversion-plan.md new file mode 100644 index 00000000..21e839e1 --- /dev/null +++ b/docs/plans/2026-07-13-001-feat-fqn-plugin-auto-conversion-plan.md @@ -0,0 +1,454 @@ +--- +title: "feat: Auto-convert bare-FQN Python plugins to isolated_venv at install time" +type: feat +date: 2026-07-13 +origin: docs/brainstorms/2026-07-13-fqn-plugin-auto-conversion-requirements.md +depth: standard +branch: feat/python_plugin_compat_0.1.x +--- + +# feat: Auto-convert bare-FQN Python plugins to isolated_venv at install time + +## Summary + +Make the `cpex` / `mcpplugins` CLI installer recognize bare-FQN Python plugins — those +whose manifest `kind` is a Python class path (e.g. `cpex_pii_filter.pii_filter.PIIFilterPlugin`) +rather than a known kind — and auto-convert them into `isolated_venv` plugins during +install. Convert the FQN into `default_config.class_name`, set `kind: isolated_venv`, +make `requirements.txt` optional throughout venv init, install the plugin package into the +venv from each channel's own source, persist the converted form to both the on-disk +manifest and `plugins/config.yaml`, and trigger venv reinstall on manifest version/hash +change including on repeat `install`. Python-installer only; no Rust changes. + +--- + +## Problem Frame + +Today the installer only drives the isolated-venv path when a manifest declares +`kind: isolated_venv` + `default_config.class_name`. Existing FQN Python plugins declare +`kind` *as* the class path with no `class_name` and often no `requirements.txt`, so they +install into the CLI's own venv and cannot be run through the Rust `PluginManager`'s +isolated-venv adapter that the 0.2.x work depends on. + +Two coupled code realities confirmed during research: +- `install_from_pypi`, `install_from_git`, and `install_from_local` all normalize their + manifest through `PluginCatalog._normalize_manifest_data` (`cpex/tools/catalog.py`), + but the monorepo path `install_folder_via_pip` branches on `manifest.kind` directly and + never calls normalize. +- `IsolatedVenvPlugin.initialize` (`cpex/framework/isolated/client.py:213`) does a raw + `self.config.config["requirements_file"]` access, and `_initialize_isolated_venv` + (`cpex/tools/catalog.py:1121`) copies a requirements file into the plugin path — both + assume `requirements.txt` exists. The venv cache key + (`_compute_requirements_hash`) is derived only from that file, so a no-requirements + plugin has a constant empty hash and never invalidates. + +This plan is **Option 1** from `crates/cpex-hosts-python/README.md`. + +--- + +## Requirements Traceability + +Origin: `docs/brainstorms/2026-07-13-fqn-plugin-auto-conversion-requirements.md` + +| Req | Description | Units | +| --- | --- | --- | +| R1 | Detect FQN kind (dotted-path shape + not a known kind; reject typos) | U1 | +| R2 | Convert FQN → `kind: isolated_venv` + `class_name`; normalize `default_configs` | U2 | +| R3 | Persist converted form to manifest + `config.yaml` | U2, U5 | +| R4 | `requirements.txt` optional throughout venv init | U3 | +| R4a | Install plugin package into venv per channel source | U4 | +| R5 | Reinstall trigger by manifest version + hash (alongside requirements hash) | U5 | +| R6 | Repeat `install` compares versions and reinstalls on mismatch | U6 | +| R7 | Regression (test-plugin) + FQN-fixture conversion + hook execution | U7 | + +--- + +## Key Technical Decisions + +**KTD1 — Converter lives in `_normalize_manifest_data`, plus a parallel monorepo hook.** +`install_from_pypi` / `install_from_git` / `install_from_local` all pass through +`_normalize_manifest_data`, so placing detection + conversion there covers three of four +channels with one change. The monorepo path (`install_folder_via_pip`) bypasses normalize +and branches on `manifest.kind`, so it gets the same conversion applied to the manifest it +loads from the catalog before the kind check. Rationale: single conversion concept, minimal +duplication, no new pre-install wrapper layer. (Alternative in Alternatives Considered.) + +**KTD2 — FQN detection is shape-based, no import.** A `kind` is an FQN-to-convert when it +is not in the known set **and** matches a dotted-path shape: 2+ dot-separated segments, +each a valid Python identifier, final segment starting uppercase (class convention). +Anything else unknown is rejected with an error naming the supported kinds. No venv or +import at detection time — importability is proven by U7's fixture, not the detector +(see origin R1). + +**KTD3 — Known-kind set is a single shared constant.** Introduce one authoritative +constant for `{builtin, native, wasm, external, isolated_venv, PDP}` and reference it from +the detector and the rejection error. Planning assumption from origin: confirm no existing +constant already encodes this before adding a new one (search `cpex/framework/constants.py` +and `models.py`); reuse if present. + +**KTD4 — `requirements_file` becomes optional via safe access + skip.** Replace the raw +`config["requirements_file"]` access with a `.get()` that tolerates absence; when absent, +venv creation and caching still run but requirements installation is skipped. Converted +plugins get no synthesized `requirements_file` — the package reaches the venv via KTD5. + +**KTD5 — Package enters the venv from the channel's resolved source (R4a).** After venv +creation, install the plugin package into the isolated venv using the same source the +channel already has: `pip install ` (pypi, test-pypi via index URL), the +`git+…` URL (git — mirrors existing `install_from_git` isolated branch), `-e ` +(local), the monorepo subdirectory URL (monorepo). When a `requirements.txt` is present it +still layers on top, unchanged for existing isolated_venv plugins. + +**KTD6 — Cache key composes manifest version+hash alongside requirements hash.** Extend the +venv cache metadata (`IsolatedVenvPlugin._save_cache_metadata` / `_is_venv_cache_valid`) to +also record and compare the plugin manifest version and a hash of the persisted +`plugin-manifest.yaml`. Cache is valid only if **both** the requirements hash and the +manifest version+hash are unchanged; either changing forces reinstall. Preserves existing +requirements-driven behavior. (see origin R5) + +**KTD7 — Repeat-install version compare against the registry's recorded version (R6).** In +the `plugin install` command (`cpex/tools/cli.py`), replace the "already installed → +return" short-circuit: when `registry.has(source)`, compare the resolved catalog/manifest +version against the registry's recorded installed version (`PluginRegistry` stores +`version` per plugin) and proceed to reinstall when they differ; otherwise no-op as today. +**Cross-cutting (intentional):** this changes repeat-`install` for all plugin kinds, not +only converted ones — flagged in the origin and noted in System-Wide Impact. + +--- + +## High-Level Technical Design + +Install-time flow for a Python plugin, showing where conversion and the reinstall gate sit: + +```mermaid +flowchart TD + A[plugin install source] --> B{registry.has source?} + B -- yes --> V{catalog version != installed version?} + V -- no --> NOOP[no-op: already installed] + V -- yes --> C + B -- no --> C[load + normalize manifest] + C --> D{kind classification} + D -- known kind --> E[existing path unchanged] + D -- FQN shape --> F[convert: kind=isolated_venv, class_name=FQN] + D -- unknown, not FQN --> R[reject: error naming supported kinds] + F --> G[persist manifest + config.yaml] + E --> G + G --> H{isolated_venv?} + H -- yes --> I[create venv] + I --> J{requirements.txt present?} + J -- yes --> K[install requirements] + J -- no --> L[skip requirements] + K --> M[install plugin pkg from channel source] + L --> M + M --> N[cache metadata: reqs hash + manifest version/hash] + H -- no --> O[install into CLI venv] +``` + +Cache validity (KTD6): reinstall when `reqs_hash changed OR manifest_version changed OR manifest_hash changed`. + +--- + +## Implementation Units + +### U1. FQN kind detection + known-kind constant + +**Goal:** Classify a manifest `kind` as known / FQN-to-convert / rejected. +**Requirements:** R1 +**Dependencies:** none +**Files:** +- `cpex/framework/constants.py` (or reuse existing constant if found — KTD3) +- `cpex/framework/models.py` or `cpex/tools/catalog.py` — detection helper (place near where conversion will consume it, U2) +- `tests/unit/cpex/tools/test_catalog.py` (or a focused new test module for the helper) + +**Approach:** Add a known-kind constant (KTD3) and a pure classification function that +returns one of `known` / `fqn` / `reject` given a `kind` string. FQN test: not in known +set AND dotted-path shape (2+ identifier segments, final segment uppercase-initial). No +import/venv. The rejection case surfaces a clear error listing supported kinds; wire the +raise at the conversion call site (U2), keeping the classifier itself side-effect-free. + +**Patterns to follow:** existing validators in `cpex/framework/models.py` (e.g. +`check_config_and_external`), module-level constants style in `cpex/framework/constants.py`. + +**Test scenarios:** +- Known kinds (`builtin`, `native`, `wasm`, `external`, `isolated_venv`, `PDP`) each classify as `known`. +- `cpex_pii_filter.pii_filter.PIIFilterPlugin` classifies as `fqn`. Covers AE-equivalent of R1. +- Single-segment `PIIFilterPlugin` (no dots) → `reject` (not a dotted path). +- Typo `isolate_venv` → `reject`. +- Lowercase-final-segment dotted path `a.b.c` → `reject` (not class-shaped). +- Empty / whitespace `kind` → `reject`. + +**Verification:** classifier returns the correct bucket for the table above with no filesystem or import side effects. + +--- + +### U2. Convert FQN manifest to isolated_venv in normalize path + +**Goal:** When a manifest's kind is FQN, rewrite it to `kind: isolated_venv` with +`default_config.class_name` set, preserving other fields; reject non-FQN unknowns. +**Requirements:** R2, R3 (in-memory conversion feeding persistence) +**Dependencies:** U1 +**Files:** +- `cpex/tools/catalog.py` — `_normalize_manifest_data` (and `_transform_manifest_data` for the monorepo/catalog-update path) +- `tests/unit/cpex/tools/test_catalog.py` + +**Approach:** In `_normalize_manifest_data`, after the existing `default_configs` → +`default_config` normalization, classify `kind` (U1). On `fqn`: move the FQN string into +`default_config["class_name"]` (do not overwrite an existing `class_name`; if present and +mismatched, prefer the explicit `class_name` and log), set `kind = "isolated_venv"`. On +`reject`: raise with the supported-kinds message. On `known`: unchanged. Apply the same +conversion in the catalog-update transform so the persisted catalog manifest reflects the +converted kind. Do not synthesize `requirements_file` here (KTD4/KTD5). + +**Patterns to follow:** existing `default_configs` normalization already in +`_normalize_manifest_data` and `_transform_manifest_data`. + +**Test scenarios:** +- FQN manifest (legacy `default_configs`, no `class_name`) → `kind == isolated_venv`, `default_config.class_name == `, other fields preserved. Covers R2. +- Manifest already `isolated_venv` with `class_name` → untouched (idempotent). +- Manifest with both an FQN kind AND a pre-existing `class_name` → keeps explicit `class_name`, kind becomes `isolated_venv`. +- Manifest with unknown non-FQN kind → raises with message naming supported kinds. +- `default_configs` (plural) present → normalized to `default_config` before conversion reads it. + +**Verification:** normalized `PluginManifest` for an FQN input is a valid isolated_venv manifest consumable by `_handle_plugin_installation` with no further edits. + +--- + +### U3. Make requirements.txt optional in venv init + +**Goal:** Venv initialization succeeds when the plugin has no `requirements.txt`. +**Requirements:** R4 +**Dependencies:** none (independent of U1/U2; enables U4) +**Files:** +- `cpex/framework/isolated/client.py` — `IsolatedVenvPlugin.initialize`, cache-hash helpers +- `cpex/tools/catalog.py` — `_initialize_isolated_venv` (the requirements-copy step) +- `tests/unit/cpex/framework/isolated/test_client.py` + +**Approach:** Replace the raw `self.config.config["requirements_file"]` access with a +tolerant lookup (default absent). When no requirements file is configured or the resolved +path does not exist: still create the venv and write cache metadata, but skip +`install_requirements`. In `_initialize_isolated_venv`, guard the requirements-copy so a +converted plugin without a requirements file does not error. `create_venv` already handles +a missing file via the empty-hash path; confirm and keep that behavior. + +**Execution note:** Add a failing test for `initialize()` with no `requirements_file` in config before changing the access, to lock the KeyError regression. + +**Patterns to follow:** existing `create_venv` / `_is_venv_cache_valid` empty-file handling in `cpex/framework/isolated/client.py`. + +**Test scenarios:** +- `initialize()` with no `requirements_file` key in config → venv created, no raise, requirements install skipped. +- `initialize()` with `requirements_file` pointing at a non-existent path → skipped gracefully, no raise. +- `initialize()` with a valid `requirements_file` → requirements installed (existing behavior preserved). +- `_initialize_isolated_venv` with a manifest lacking a requirements file → no copy attempted, no error. + +**Verification:** an isolated_venv plugin with no requirements initializes its venv without error; a plugin with requirements still installs them. + +--- + +### U4. Install plugin package into venv from channel source + +**Goal:** Ensure the converted plugin's FQN module is importable in the isolated venv by +installing the package from each channel's resolved source. +**Requirements:** R4a +**Dependencies:** U2, U3 +**Files:** +- `cpex/tools/catalog.py` — `install_from_pypi`, `install_from_git`, `install_from_local`, `install_folder_via_pip` (monorepo), and/or `_handle_plugin_installation` +- `tests/unit/cpex/tools/test_catalog.py` + +**Approach:** After venv init for an isolated_venv plugin that has no self-referencing +requirements, install the plugin package into the venv using the channel source: +`pip install ` for pypi/test-pypi (test-pypi via `--index-url`), the +`git+…` URL for git (the git path already does this — extend it to the converted case), +`-e ` for local, the monorepo subdirectory URL for monorepo. Centralize the +"install into venv python" step so all channels share it (venv python resolved via +`_get_venv_python_executable`). When a requirements file is present, this layers after +requirements install. + +**Patterns to follow:** existing isolated-venv install in `install_from_git` +(`cpex/tools/catalog.py:1542-1555`) and venv-python resolution `_get_venv_python_executable`. + +**Test scenarios:** +- pypi channel, converted no-requirements plugin → package installed into venv with the version constraint applied (subprocess args asserted; pip mocked). +- test-pypi channel → install invoked with the test index URL. +- git channel → package installed into venv from the `git+` URL (existing behavior holds for converted plugins). +- local channel → editable install (`-e`) into venv. +- monorepo channel → package installed into venv from the subdirectory source. +- Plugin *with* a requirements file → requirements install AND package install both occur (layering). + +**Verification:** after install, the plugin's FQN module resolves inside `plugins//.venv` (asserted end-to-end in U7). + +--- + +### U5. Persist converted manifest + manifest-based cache key + +**Goal:** Persist the converted form to `plugins//plugin-manifest.yaml` and +`plugins/config.yaml`, and extend the venv cache key to include manifest version + hash. +**Requirements:** R3, R5 +**Dependencies:** U2, U3 +**Files:** +- `cpex/tools/catalog.py` — `_persist_manifest`, `_finalize_plugin_installation`, `_initialize_isolated_venv` +- `cpex/tools/cli.py` — `update_plugins_config_yaml` (converted config lands in config.yaml) +- `cpex/framework/isolated/client.py` — `_save_cache_metadata`, `_is_venv_cache_valid`, cache-hash helpers +- `tests/unit/cpex/tools/test_catalog.py` +- `tests/unit/cpex/framework/isolated/test_client.py` + +**Approach:** Confirm the converted manifest is written under `plugins//plugin-manifest.yaml` +(the stable diff record) in addition to the catalog copy, and that the converted +`PluginConfig` (kind=isolated_venv + class_name) flows into `plugins/config.yaml` via the +existing `update_plugins_config_yaml`. Extend cache metadata (KTD6) to record +`manifest_version` and `manifest_hash` (hash of the persisted manifest). `_is_venv_cache_valid` +returns valid only when requirements hash AND manifest version AND manifest hash all match; +any mismatch invalidates. + +**Test scenarios:** +- Converting + installing an FQN plugin writes `plugins//plugin-manifest.yaml` with `kind: isolated_venv` + `class_name`. Covers R3. +- The generated entry in `plugins/config.yaml` carries `kind: isolated_venv` and `config.class_name`. Covers R3. +- Cache valid when manifest version+hash and requirements hash all unchanged → no reinstall. +- Manifest version bump with identical requirements → cache invalid → reinstall triggered. Covers R5. +- Manifest content change at same version → manifest hash differs → cache invalid. +- No-requirements plugin: manifest version+hash is the sole invalidation signal (requirements hash constant) → version bump still reinstalls. + +**Verification:** a version bump to a no-requirements converted plugin invalidates the venv cache; an unchanged manifest reuses the cached venv. + +--- + +### U6. Repeat-install version compare + +**Goal:** Repeat `install` of an already-registered plugin reinstalls when the version +differs, instead of a no-op. +**Requirements:** R6 +**Dependencies:** U5 +**Files:** +- `cpex/tools/cli.py` — `plugin` command (the `registry.has(source)` short-circuit) and/or `install` +- `tests/unit/cpex/tools/test_cli.py` + +**Approach:** Replace the early `return` when `registry.has(source)` with a version +comparison: resolve the target version (catalog/manifest for the requested source) and +compare against the registry's recorded installed version (`PluginRegistry` stores +`version`). Equal → keep the "already installed" no-op message. Different → fall through to +the normal install path (which now re-runs conversion + venv reinstall via U5's cache key). +Note the cross-cutting effect in the command help / release notes. + +**Test scenarios:** +- Repeat install, same version already registered → no-op, "already installed" message, no reinstall side effects. +- Repeat install, catalog version greater than registered version → proceeds to install/reinstall. +- Repeat install, source not yet registered → normal install (unchanged). +- Applies uniformly to a non-converted (`isolated_venv` or native) plugin — version compare is kind-agnostic. Covers the R6 cross-cutting note. + +**Verification:** installing a plugin, bumping its catalog version, and re-running `install` triggers a reinstall; re-running at the same version does not. + +--- + +### U7. Acceptance: regression + FQN-fixture conversion with hook execution + +**Goal:** Prove existing isolated_venv install is unregressed and the new FQN conversion +path works end-to-end through the isolated worker. +**Requirements:** R7 +**Dependencies:** U1–U6 +**Files:** +- `tests/unit/cpex/fixtures/plugins/isolated/test_plugin/` (existing regression fixture) +- `tests/unit/cpex/fixtures/plugins/` — new synthetic bare-FQN fixture (unknown FQN `kind`, no `requirements.txt`, a plugin class + `plugin-manifest.yaml`) +- `tests/unit/cpex/framework/isolated/test_integration.py` and/or `tests/unit/cpex/tools/test_catalog.py` + +**Approach:** (1) Regression: existing `cpex-test-plugin` isolated fixture installs, +initializes venv, and loads unchanged. (2) Conversion: a synthetic FQN fixture (kind is a +class path, no requirements.txt) installs via a real-ish channel (local/`-e` against the +fixture dir is the cheapest real path), auto-converts to `isolated_venv` + `class_name`, +gets its package into `plugins//.venv`, is persisted to both the manifest and +`config.yaml`, and **executes a hook through the isolated worker** returning the expected +result. Use controlled fixtures (no live third-party package). + +**Execution note:** Start from the conversion acceptance test as a failing end-to-end test that drives U1–U6 integration. + +**Test scenarios:** +- Regression: `cpex-test-plugin` fixture installs and loads with no behavior change. Covers R7(1). +- Conversion: FQN fixture (no requirements) → persisted manifest + config.yaml show `isolated_venv` + `class_name`; venv exists at `plugins//.venv`; FQN module importable. Covers R7(2), R3, R4a. +- Hook execution: a hook invoked on the converted plugin through the isolated worker returns the expected `PluginResult`. Covers R7(2). +- Reinstall: bump the FQN fixture manifest version, re-install → venv reinstalled (ties U5 + U6 together). + +**Verification:** full test suite green; the FQN fixture runs a hook via the isolated worker after auto-conversion, and the test-plugin regression passes unchanged. + +--- + +## Scope Boundaries + +**In scope:** Python installer changes on `feat/python_plugin_compat_0.1.x` covering R1–R7. + +**Deferred for later** (from origin): +- README **Option 2** (`migration.md` manual-edit path). +- An explicit `--force` / `upgrade` action — superseded by U6's version compare. + +**Outside this product's identity / this branch** (from origin): +- Any Rust-side changes (`crates/cpex-hosts-python`, 0.2.x). Correctness of the Rust + runtime consuming the converted `config.yaml` is validated separately in that work. + +**Deferred to Follow-Up Work** (plan-local): +- Plugins distributed only as loose source with no installable package (out of R4a's + channel-source model). + +--- + +## System-Wide Impact + +- **Repeat-install behavior changes for ALL plugin kinds** (U6/KTD7), not just converted + FQN plugins: a repeat `install` becomes install-with-upgrade-on-version-change rather + than an unconditional no-op. Intentional per origin; call out in command help and any + release notes so operators relying on the no-op are not surprised. +- **`plugins/config.yaml` and persisted manifests** gain converted `isolated_venv` entries + for previously-FQN plugins; the Rust `PluginManager` reads these unchanged. +- **Venv cache invalidation** now also keys on manifest version/hash — a manifest edit at + the same version now invalidates where before only requirements changes did. + +--- + +## Alternatives Considered + +- **Standalone pre-install conversion pass wrapping all channels** (instead of KTD1's + normalize-path placement). Rejected: adds a new layer duplicating the manifest-load + step, and three of four channels already funnel through `_normalize_manifest_data`. The + monorepo path is the only bypass and is handled with a small parallel hook. +- **Synthesize a one-line `requirements.txt` for converted plugins** (origin's alternative + to R4a). Rejected as the default: reusing each channel's resolved source is more direct, + avoids inventing a file the user never wrote, and matches the existing git isolated + install; the synthesized-file approach remains a fallback if a channel's source proves + hard to install directly. + +--- + +## Risks & Dependencies + +- **Detection false-positives/negatives (R1).** A class-path-shaped but non-plugin `kind` + could be misclassified. Mitigated by the strict shape rule (KTD2) and U7's real hook + execution catching a bad conversion; importability is proven at test time, not asserted + by the detector. +- **Cache-key composition regressions (KTD6).** Changing `_is_venv_cache_valid` risks + invalidating healthy caches for existing isolated_venv plugins. Mitigated by U5 scenarios + asserting unchanged-manifest cache reuse and by keeping the requirements-hash check intact. +- **Assumption — known-kind constant.** Confirm during U1 whether an authoritative + known-kind constant already exists (`cpex/framework/constants.py`, `models.py`) and reuse + it rather than introducing a divergent list. +- **Assumption — FQN package is installable from its channel source (R4a).** Loose-source + plugins are deferred. + +--- + +## Open Questions (deferred to implementation) + +- Exact home of the detection helper (models vs. catalog module) — place it where U2 + consumes it with least import coupling. +- Whether the manifest hash in KTD6 hashes the raw file bytes or the normalized model dump + — settle when wiring `_save_cache_metadata`; prefer hashing the persisted file for a + stable on-disk diff signal. +- Whether U4's per-channel install-into-venv is best centralized in + `_handle_plugin_installation` or kept per-channel — decide once the git path's existing + isolated install is refactored to be shared. + +--- + +## Sources & Research + +- Origin requirements: `docs/brainstorms/2026-07-13-fqn-plugin-auto-conversion-requirements.md` +- `crates/cpex-hosts-python/README.md` — Option 1. +- Installer: `cpex/tools/cli.py` (`plugin`, `install`, `update_plugins_config_yaml`, `registry.has` short-circuit). +- Catalog: `cpex/tools/catalog.py` (`_normalize_manifest_data`, `_transform_manifest_data`, `_handle_plugin_installation`, `_initialize_isolated_venv`, `install_from_pypi`/`install_from_git`/`install_from_local`/`install_folder_via_pip`, `_finalize_plugin_installation`, `_persist_manifest`, `_get_venv_python_executable`). +- Isolated client: `cpex/framework/isolated/client.py` (`initialize`, `create_venv`, `_compute_requirements_hash`, `_is_venv_cache_valid`, `_save_cache_metadata`). +- Models/registry: `cpex/framework/models.py` (`PluginManifest`, `create_instance_config`), `cpex/tools/plugin_registry.py` (`PluginRegistry.has`, per-plugin `version`). +- Test conventions: `tests/unit/cpex/tools/test_catalog.py`, `tests/unit/cpex/tools/test_cli.py`, `tests/unit/cpex/framework/isolated/{test_client.py,test_integration.py,conftest.py}`, fixture at `tests/unit/cpex/fixtures/plugins/isolated/test_plugin/`. diff --git a/tests/unit/cpex/fixtures/plugins/isolated/fqn_plugin/plugin-manifest.yaml b/tests/unit/cpex/fixtures/plugins/isolated/fqn_plugin/plugin-manifest.yaml new file mode 100644 index 00000000..2d5e866d --- /dev/null +++ b/tests/unit/cpex/fixtures/plugins/isolated/fqn_plugin/plugin-manifest.yaml @@ -0,0 +1,12 @@ +# Bare-FQN plugin manifest: kind is a Python class path, NOT a known kind. +# No requirements.txt. Exercises installer FQN auto-conversion (U7). +name: fqn-plugin +kind: fqn_plugin.plugin.FqnPlugin +description: A synthetic bare-FQN plugin fixture +author: habeck +version: 0.1.0 +tags: + - test +available_hooks: + - tool_pre_invoke +default_configs: {} diff --git a/tests/unit/cpex/fixtures/plugins/isolated/fqn_plugin/plugin.py b/tests/unit/cpex/fixtures/plugins/isolated/fqn_plugin/plugin.py new file mode 100644 index 00000000..837caeb9 --- /dev/null +++ b/tests/unit/cpex/fixtures/plugins/isolated/fqn_plugin/plugin.py @@ -0,0 +1,35 @@ +"""A synthetic bare-FQN plugin fixture (no requirements.txt). + +Copyright 2025 +SPDX-License-Identifier: Apache-2.0 +Authors: habeck + +Used by U7 acceptance tests: a plugin whose manifest declares ``kind`` as a +class path rather than a known kind, exercising the installer's FQN +auto-conversion to isolated_venv. +""" + +import logging + +from cpex.framework import ( + Plugin, + PluginConfig, + PluginContext, + ToolPreInvokePayload, + ToolPreInvokeResult, +) + +logger = logging.getLogger(__name__) + + +class FqnPlugin(Plugin): + """A minimal plugin referenced by its fully-qualified class path.""" + + def __init__(self, config: PluginConfig): + """Entry init block for the plugin.""" + super().__init__(config) + + async def tool_pre_invoke(self, payload: ToolPreInvokePayload, context: PluginContext) -> ToolPreInvokeResult: + """Allow the tool invocation to proceed.""" + logger.info("FqnPlugin: tool_pre_invoke") + return ToolPreInvokeResult(continue_processing=True) diff --git a/tests/unit/cpex/framework/isolated/test_client.py b/tests/unit/cpex/framework/isolated/test_client.py index d60a3089..45549cc0 100644 --- a/tests/unit/cpex/framework/isolated/test_client.py +++ b/tests/unit/cpex/framework/isolated/test_client.py @@ -89,6 +89,25 @@ async def test_create_venv_success(self, mock_builder_class, plugin, tmp_path): mock_builder_class.assert_called_once() mock_builder.create.assert_called_once_with(str(venv_path)) + @pytest.mark.asyncio + @patch("cpex.framework.isolated.client.venv.EnvBuilder") + async def test_create_venv_reuses_cache_with_no_requirements(self, mock_builder_class, plugin, tmp_path): + """create_venv reuses a cached venv for a no-requirements plugin (Finding #1 fix). + + With no requirements_file, the manifest version+hash is the sole cache + signal. A valid cache must be honored — the venv must NOT be rebuilt. + """ + venv_path = plugin.plugin_path / ".venv" + venv_path.mkdir(parents=True, exist_ok=True) + # Seed valid cache metadata reflecting the current (no-requirements) state. + plugin._save_cache_metadata(str(venv_path), None) + + result = await plugin.create_venv(str(venv_path), requirements_file=None, use_cache=True) + + # Cache hit -> returns False and does NOT rebuild the venv. + assert result is False + mock_builder_class.assert_not_called() + @pytest.mark.asyncio @patch("cpex.framework.isolated.client.venv.EnvBuilder") async def test_create_venv_failure(self, mock_builder_class, plugin, tmp_path): @@ -117,6 +136,74 @@ async def test_initialize_success(self, mock_create_venv, mock_comm_class, plugi mock_comm.install_requirements.assert_called_once() assert plugin.comm is not None + @pytest.fixture + def config_no_requirements(self, tmp_path): + """A plugin config with no requirements_file (converted FQN plugin).""" + plugin_dir = tmp_path / "test_plugin" + plugin_dir.mkdir(parents=True, exist_ok=True) + config_dict = { + "name": "test_plugin", + "kind": "isolated_venv", + "description": "Test plugin", + "version": "1.0.0", + "author": "Test", + "hooks": ["tool_pre_invoke"], + "config": {"class_name": "test_plugin.TestPlugin"}, # no requirements_file + } + return PluginConfig(**config_dict) + + @pytest.fixture + def plugin_no_requirements(self, config_no_requirements, tmp_path): + instance = IsolatedVenvPlugin(config_no_requirements, plugin_dirs=[tmp_path]) + instance.plugin_path = tmp_path / "test_plugin" + return instance + + @pytest.mark.asyncio + @patch("cpex.framework.isolated.client.VenvProcessCommunicator") + @patch.object(IsolatedVenvPlugin, "create_venv") + async def test_initialize_without_requirements_file( + self, mock_create_venv, mock_comm_class, plugin_no_requirements + ): + """initialize() with no requirements_file creates the venv and skips install.""" + mock_create_venv.return_value = True # newly created venv + mock_comm = MagicMock() + mock_comm_class.return_value = mock_comm + + # Must not raise (previously a KeyError on config['requirements_file']). + await plugin_no_requirements.initialize() + + mock_create_venv.assert_called_once() + # No requirements file -> install_requirements is skipped. + mock_comm.install_requirements.assert_not_called() + assert plugin_no_requirements.comm is not None + + @pytest.mark.asyncio + @patch("cpex.framework.isolated.client.VenvProcessCommunicator") + @patch.object(IsolatedVenvPlugin, "create_venv") + async def test_initialize_requirements_file_missing_on_disk( + self, mock_create_venv, mock_comm_class, plugin + ): + """initialize() when the configured requirements file doesn't exist skips install gracefully.""" + # plugin's config declares requirements.txt but remove the file from disk. + req = plugin.plugin_path / "requirements.txt" + if req.exists(): + req.unlink() + mock_create_venv.return_value = True + mock_comm = MagicMock() + mock_comm_class.return_value = mock_comm + + await plugin.initialize() + + mock_comm.install_requirements.assert_not_called() + assert plugin.comm is not None + + def test_compute_requirements_hash_none_is_empty_digest(self, plugin): + """A None requirements file hashes to the empty-content digest.""" + import hashlib + + expected = hashlib.sha256(b"").hexdigest() + assert plugin._compute_requirements_hash(None) == expected + @pytest.mark.asyncio @patch("cpex.framework.isolated.client.get_hook_registry") async def test_invoke_hook_unregistered_hook_type(self, mock_get_registry, plugin, plugin_context): @@ -491,13 +578,15 @@ def test_is_venv_cache_valid_success(self, plugin, tmp_path): req_file = tmp_path / "requirements.txt" req_file.write_text("pytest==7.0.0\n") - # Create metadata with correct hash + # Create metadata with correct hashes, including manifest version + hash. req_hash = plugin._compute_requirements_hash(str(req_file)) metadata_path = plugin._get_cache_metadata_path(str(venv_path)) metadata = { "venv_path": str(venv_path), "requirements_file": str(req_file), "requirements_hash": req_hash, + "manifest_version": plugin.config.version, + "manifest_hash": plugin._compute_manifest_hash(), "python_version": f"{sys.version_info.major}.{sys.version_info.minor}.{sys.version_info.micro}", } metadata_path.write_text(json.dumps(metadata)) @@ -556,6 +645,96 @@ def test_save_cache_metadata_nonexistent_requirements(self, plugin, tmp_path): assert metadata["requirements_file"] is None + def test_save_cache_metadata_records_manifest_version_and_hash(self, plugin, tmp_path): + """Cache metadata records manifest version and hash (U5).""" + venv_path = tmp_path / ".venv" + venv_path.mkdir() + + plugin._save_cache_metadata(str(venv_path), None) + + with open(plugin._get_cache_metadata_path(str(venv_path))) as f: + metadata = json.load(f) + + assert metadata["manifest_version"] == plugin.config.version + assert metadata["manifest_hash"] == plugin._compute_manifest_hash() + + def test_cache_invalid_on_manifest_version_change(self, plugin, tmp_path): + """A manifest version bump invalidates the cache even with same requirements (U5).""" + venv_path = tmp_path / ".venv" + venv_path.mkdir() + + # Save metadata reflecting the current state. + plugin._save_cache_metadata(str(venv_path), None) + assert plugin._is_venv_cache_valid(str(venv_path), None) is True + + # Simulate a version bump by rewriting the metadata's manifest_version. + metadata_path = plugin._get_cache_metadata_path(str(venv_path)) + metadata = json.loads(metadata_path.read_text()) + metadata["manifest_version"] = "0.0.1-old" + metadata_path.write_text(json.dumps(metadata)) + + assert plugin._is_venv_cache_valid(str(venv_path), None) is False + + def test_cache_invalid_on_manifest_hash_change(self, plugin, tmp_path): + """A manifest content change (same version) invalidates the cache (U5).""" + venv_path = tmp_path / ".venv" + venv_path.mkdir() + + plugin._save_cache_metadata(str(venv_path), None) + assert plugin._is_venv_cache_valid(str(venv_path), None) is True + + # Write the plugin's manifest (per-class-name path) so the current hash + # differs from the cached empty digest. + plugin._manifest_path().write_text("kind: isolated_venv\nname: p\n") + + assert plugin._is_venv_cache_valid(str(venv_path), None) is False + + def test_compute_manifest_hash_empty_when_absent(self, plugin): + """Manifest hash is the empty-content digest when no manifest file exists.""" + import hashlib + + # plugin.plugin_path has no manifest by default. + assert plugin._compute_manifest_hash() == hashlib.sha256(b"").hexdigest() + + def test_cache_valid_when_manifest_signals_absent(self, plugin, tmp_path): + """Pre-upgrade metadata (no manifest_version/hash keys) stays valid. + + Regression (R5): metadata written by an earlier CLI lacks the manifest + signal keys. Reading .get() -> None as a mismatch would wipe and rebuild + every existing isolated_venv on first run after upgrade. A *missing* key + must be treated as "no signal", not a change. + """ + venv_path = tmp_path / ".venv" + venv_path.mkdir() + + # Save current metadata, then strip the manifest signal keys to mimic + # metadata produced by a CLI that predates U5. + plugin._save_cache_metadata(str(venv_path), None) + metadata_path = plugin._get_cache_metadata_path(str(venv_path)) + metadata = json.loads(metadata_path.read_text()) + metadata.pop("manifest_version", None) + metadata.pop("manifest_hash", None) + metadata_path.write_text(json.dumps(metadata)) + + # Requirements hash still matches, and the absent signals are ignored: + # the cache remains valid (no forced reprovision). + assert plugin._is_venv_cache_valid(str(venv_path), None) is True + + def test_manifest_path_keyed_on_full_class_name(self, plugin): + """The persisted manifest filename is keyed on the full class name (#4). + + Plugins sharing a package share plugin_path (and its venv); the manifest + filename must be unique per class so one plugin's install does not + invalidate another's cache hash. + """ + from cpex.framework.utils import manifest_filename_for_class + + expected_name = manifest_filename_for_class("test_plugin.TestPlugin") + assert plugin._manifest_path().name == expected_name + assert plugin._manifest_path().parent == plugin.plugin_path + # Two plugins in the same package resolve to distinct manifest files. + assert manifest_filename_for_class("pkg.a.PluginA") != manifest_filename_for_class("pkg.b.PluginB") + @pytest.mark.asyncio @patch("cpex.framework.isolated.client.venv.EnvBuilder") @patch("cpex.framework.isolated.client.shutil.rmtree") diff --git a/tests/unit/cpex/framework/isolated/test_integration.py b/tests/unit/cpex/framework/isolated/test_integration.py index a7501a65..9fdc3dbf 100644 --- a/tests/unit/cpex/framework/isolated/test_integration.py +++ b/tests/unit/cpex/framework/isolated/test_integration.py @@ -389,4 +389,170 @@ async def test_isolated_plugin_violation_handling(self, mock_create_venv, mock_c assert result.violation is not None +class TestFqnAutoConversionAcceptance: + """U7 acceptance: regression + bare-FQN conversion end-to-end (R7).""" + + FQN_MANIFEST = { + "name": "fqn-plugin", + "kind": "fqn_plugin.plugin.FqnPlugin", # bare FQN, not a known kind + "description": "A synthetic bare-FQN plugin fixture", + "author": "habeck", + "version": "0.1.0", + "tags": ["test"], + "available_hooks": ["tool_pre_invoke"], + "default_configs": {}, # legacy plural key, no requirements_file + } + + @pytest.mark.asyncio + @patch("cpex.framework.isolated.client.VenvProcessCommunicator") + @patch.object(IsolatedVenvPlugin, "create_venv") + async def test_regression_existing_isolated_plugin_still_loads( + self, mock_create_venv, mock_comm_class, tmp_path + ): + """Regression: an already-isolated_venv plugin initializes and invokes unchanged (R7.1).""" + mock_create_venv.return_value = None + mock_comm = MagicMock() + mock_comm.send_task.return_value = { + "continue_processing": True, + "modified_payload": None, + "violation": None, + "metadata": {}, + } + mock_comm_class.return_value = mock_comm + + config = PluginConfig( + name="test_plugin", + kind="isolated_venv", + description="Test plugin", + version="1.0.0", + author="Test", + hooks=["tool_pre_invoke"], + config={"class_name": "test_plugin.TestPlugin", "requirements_file": "requirements.txt"}, + ) + resolved = (tmp_path / "xplugins").resolve() + (resolved / "test_plugin").mkdir(parents=True, exist_ok=True) + plugin = IsolatedVenvPlugin(config, plugin_dirs=[resolved]) + + with patch("cpex.framework.isolated.client.get_hook_registry") as mock_registry: + from cpex.framework.hooks.tools import ToolPreInvokeResult + from cpex.framework.models import PluginContext + + mock_reg = MagicMock() + mock_reg.get_result_type.return_value = ToolPreInvokeResult + mock_reg.json_to_result.return_value = ToolPreInvokeResult(continue_processing=True) + mock_registry.return_value = mock_reg + + await plugin.initialize() + context = PluginContext(global_context=GlobalContext(request_id="req-1")) + result = await plugin.invoke_hook( + "tool_pre_invoke", ToolPreInvokePayload(name="t", args={}), context + ) + assert result.continue_processing is True + + def test_fqn_manifest_converts_and_persists(self, tmp_path, monkeypatch): + """A bare-FQN manifest converts to isolated_venv + class_name and persists to plugin dir (R2, R3).""" + from cpex.tools.catalog import PluginCatalog + + monkeypatch.setenv("PLUGINS_GITHUB_TOKEN", "test_token") + with patch("cpex.tools.catalog.Github"): + catalog = PluginCatalog() + catalog.plugin_folder = str(tmp_path / "plugins") + + manifest = catalog._normalize_manifest_data(dict(self.FQN_MANIFEST), "fqn-plugin", None) + + # Converted in memory. + assert manifest.kind == "isolated_venv" + assert manifest.default_config["class_name"] == "fqn_plugin.plugin.FqnPlugin" + + # Persisted under plugins// with a per-full-class-name + # filename (so multi-plugin packages don't collide — see #4). + from cpex.framework.utils import manifest_filename_for_class + + written_path = catalog._persist_manifest_to_plugin_dir(manifest) + expected = ( + tmp_path / "plugins" / "fqn_plugin" / manifest_filename_for_class("fqn_plugin.plugin.FqnPlugin") + ) + assert written_path == expected + persisted = yaml.safe_load(expected.read_text()) + assert persisted["kind"] == "isolated_venv" + assert persisted["default_config"]["class_name"] == "fqn_plugin.plugin.FqnPlugin" + + @pytest.mark.asyncio + @patch("cpex.framework.isolated.client.VenvProcessCommunicator") + @patch.object(IsolatedVenvPlugin, "create_venv") + async def test_converted_fqn_plugin_executes_hook(self, mock_create_venv, mock_comm_class, tmp_path): + """A converted FQN plugin (no requirements) initializes its venv and executes a hook (R4, R7.2).""" + mock_create_venv.return_value = True # newly created venv, no requirements to install + mock_comm = MagicMock() + mock_comm.send_task.return_value = { + "continue_processing": True, + "modified_payload": None, + "violation": None, + "metadata": {}, + } + mock_comm_class.return_value = mock_comm + + # Config as it would appear after conversion: isolated_venv + class_name, no requirements_file. + config = PluginConfig( + name="fqn-plugin", + kind="isolated_venv", + description="A synthetic bare-FQN plugin fixture", + version="0.1.0", + author="habeck", + hooks=["tool_pre_invoke"], + config={"class_name": "fqn_plugin.plugin.FqnPlugin"}, + ) + resolved = (tmp_path / "xplugins").resolve() + (resolved / "fqn_plugin").mkdir(parents=True, exist_ok=True) + plugin = IsolatedVenvPlugin(config, plugin_dirs=[resolved]) + + with patch("cpex.framework.isolated.client.get_hook_registry") as mock_registry: + from cpex.framework.hooks.tools import ToolPreInvokeResult + from cpex.framework.models import PluginContext + + mock_reg = MagicMock() + mock_reg.get_result_type.return_value = ToolPreInvokeResult + mock_reg.json_to_result.return_value = ToolPreInvokeResult(continue_processing=True) + mock_registry.return_value = mock_reg + + # Must initialize without a requirements file (no KeyError) and skip install. + await plugin.initialize() + mock_comm.install_requirements.assert_not_called() + + context = PluginContext(global_context=GlobalContext(request_id="req-2")) + result = await plugin.invoke_hook( + "tool_pre_invoke", ToolPreInvokePayload(name="t", args={}), context + ) + assert result.continue_processing is True + + def test_version_bump_invalidates_converted_plugin_cache(self, tmp_path): + """Bumping a converted plugin's manifest version invalidates its venv cache (U5 + U6 tie-in).""" + config = PluginConfig( + name="fqn-plugin", + kind="isolated_venv", + description="d", + version="0.1.0", + author="habeck", + hooks=["tool_pre_invoke"], + config={"class_name": "fqn_plugin.plugin.FqnPlugin"}, + ) + resolved = (tmp_path / "xplugins").resolve() + (resolved / "fqn_plugin").mkdir(parents=True, exist_ok=True) + plugin = IsolatedVenvPlugin(config, plugin_dirs=[resolved]) + + venv_path = plugin.plugin_path / ".venv" + venv_path.mkdir(parents=True, exist_ok=True) + plugin._save_cache_metadata(str(venv_path), None) + assert plugin._is_venv_cache_valid(str(venv_path), None) is True + + # Simulate a version bump recorded in the metadata being stale. + metadata_path = plugin._get_cache_metadata_path(str(venv_path)) + import json + + meta = json.loads(metadata_path.read_text()) + meta["manifest_version"] = "0.0.9" + metadata_path.write_text(json.dumps(meta)) + assert plugin._is_venv_cache_valid(str(venv_path), None) is False + + # Made with Bob diff --git a/tests/unit/cpex/framework/isolated/test_worker.py b/tests/unit/cpex/framework/isolated/test_worker.py index 3dcfb686..c2d8b3d1 100644 --- a/tests/unit/cpex/framework/isolated/test_worker.py +++ b/tests/unit/cpex/framework/isolated/test_worker.py @@ -77,6 +77,10 @@ async def test_process_task_load_and_run_hook_success( mock_plugin_instance.tool_post_invoke = AsyncMock() mock_plugin_instance.tool_exception = AsyncMock() mock_plugin_instance.tool_cleanup = AsyncMock() + # json_to_payload is a synchronous method; without this override the + # AsyncMock parent would auto-create it as an AsyncMock, and process_task + # calls it without awaiting (worker.py) — leaking an unawaited coroutine. + mock_plugin_instance.json_to_payload = MagicMock() mock_plugin_class = MagicMock(return_value=mock_plugin_instance) mock_module = MagicMock() @@ -174,6 +178,102 @@ async def test_process_task_with_different_hook_types( assert result is not None self.cleanup_mock_plugin_dirs() + @pytest.mark.asyncio + @patch("cpex.framework.isolated.worker.import_module") + async def test_process_task_deserializes_payload_to_typed_object(self, mock_import, mock_plugin_dirs): + """Regression: the worker must reconstruct the typed payload before invoking the plugin. + + The client serializes the payload with ``model_dump(mode="json")``, so it + arrives at the worker as a plain dict. Previously the worker passed that + dict straight to the plugin hook, which then failed with + ``AttributeError("'dict' object has no attribute 'args'")`` the moment the + hook touched ``payload.args``. This test drives a real Plugin subclass + through ``process_task`` and asserts the hook receives a genuine + ``ToolPreInvokePayload`` with a working ``.args`` attribute. + """ + from cpex.framework.base import Plugin + from cpex.framework.hooks.tools import ToolPreInvokePayload, ToolPreInvokeResult + + received = {} + + class RealTypedPlugin(Plugin): + """Minimal real plugin that asserts payload typing at runtime.""" + + async def tool_pre_invoke(self, payload, context, extensions=None): + # Would raise AttributeError before the fix, when payload is a dict. + received["type"] = type(payload) + received["args"] = payload.args + received["name"] = payload.name + return ToolPreInvokeResult(continue_processing=True) + + mock_module = MagicMock() + mock_module.RealTypedPlugin = RealTypedPlugin + mock_import.return_value = mock_module + + config_dict = {"name": "typed_plugin", "kind": "isolated_venv", "config": {}} + task_data = { + "task_type": "load_and_run_hook", + "config": json.dumps(config_dict), + "plugin_dirs": mock_plugin_dirs, + "class_name": "typed_plugin.RealTypedPlugin", + "hook_type": "tool_pre_invoke", + # Payload exactly as the client sends it: model_dump(mode="json") output. + "payload": {"name": "web_search", "args": {"query": "CPEX framework"}}, + "context": {"state": {}, "global_context": {"request_id": "req-123"}, "metadata": {}}, + } + tp = TaskProcessor() + result = await process_task(task_data, tp) + + assert result is not None + assert result.continue_processing is True + # The hook must have received a typed payload, not a dict. + assert received["type"] is ToolPreInvokePayload + assert received["name"] == "web_search" + assert received["args"] == {"query": "CPEX framework"} + self.cleanup_mock_plugin_dirs() + + @pytest.mark.asyncio + @patch("cpex.framework.isolated.worker.import_module") + @patch("cpex.framework.isolated.worker.PluginExecutor") + async def test_process_task_none_payload_passes_through( + self, mock_executor_class, mock_import, mock_plugin_dirs + ): + """A None payload must be forwarded as None, not run through json_to_payload.""" + mock_plugin_instance = AsyncMock() + mock_plugin_instance.initialize = AsyncMock() + mock_plugin_instance.json_to_payload = MagicMock() + mock_plugin_class = MagicMock(return_value=mock_plugin_instance) + + mock_module = MagicMock() + mock_module.TestPlugin = mock_plugin_class + mock_import.return_value = mock_module + + mock_executor = MagicMock() + mock_result = MagicMock() + mock_executor.execute_plugin = AsyncMock(return_value=mock_result) + mock_executor_class.return_value = mock_executor + + config_dict = {"name": "test_plugin", "kind": "isolated_venv", "config": {}} + task_data = { + "task_type": "load_and_run_hook", + "config": json.dumps(config_dict), + "plugin_dirs": mock_plugin_dirs, + "class_name": "test_plugin.TestPlugin", + "hook_type": "tool_pre_invoke", + "payload": None, + "context": {"state": {}, "global_context": {"request_id": "req-123"}, "metadata": {}}, + } + tp = TaskProcessor() + result = await process_task(task_data, tp) + + assert result is not None + # json_to_payload must not be invoked for a None payload. + mock_plugin_instance.json_to_payload.assert_not_called() + # execute_plugin must have been called with payload=None. + _, call_kwargs = mock_executor.execute_plugin.call_args + assert call_kwargs["payload"] is None + self.cleanup_mock_plugin_dirs() + @pytest.mark.asyncio async def test_process_task_unknown_task_type(self): """Test processing task with unknown task type.""" @@ -199,6 +299,9 @@ async def test_process_task_with_metadata( mock_plugin_instance.prompt_post_fetch = AsyncMock() mock_plugin_instance.tool_exception = AsyncMock() mock_plugin_instance.tool_cleanup = AsyncMock() + # json_to_payload is synchronous — keep it a MagicMock so the AsyncMock + # parent doesn't auto-create it as a coroutine that process_task never awaits. + mock_plugin_instance.json_to_payload = MagicMock() mock_plugin_class = MagicMock(return_value=mock_plugin_instance) @@ -333,7 +436,7 @@ async def test_main_unexpected_exception(self, mock_process_task, mock_print, mo output_data = json.loads(printed_output) assert output_data["status"] == "error" assert "Unexpected error: Unexpected error occurred" in output_data["message"] - assert output_data["request_id"] == "unknown" + assert output_data["request_id"] == "req-789" @pytest.mark.asyncio @patch("sys.stdin") @@ -448,5 +551,68 @@ async def test_main_multiple_tasks(self, mock_process_task, mock_print, mock_std assert mock_process_task.call_count == 2 assert mock_print.call_count == 2 + @pytest.mark.asyncio + @patch("sys.stdin") + @patch("builtins.print") + @patch("cpex.framework.isolated.worker.process_task") + async def test_main_error_uses_current_request_id_not_stale( + self, mock_process_task, mock_print, mock_stdin + ): + """An error on task B must carry B's request_id, never a stale A id. + + Regression: request_id was a main()-local reused across loop iterations, + so an error emitted before/without re-setting it could carry the prior + request's id. venv_comm demuxes strictly on request_id, so a stale id + misdelivers the error or hangs the real caller until timeout. + """ + task_a = {"task_type": "info", "request_id": "req-A"} + task_b = {"task_type": "info", "request_id": "req-B"} + mock_stdin.readline.side_effect = [ + json.dumps(task_a) + "\n", + json.dumps(task_b) + "\n", + "", # EOF + ] + + ok = MagicMock() + ok.model_dump.return_value = {"status": "success"} + # First task succeeds; second raises before a response is built. + mock_process_task.side_effect = [ok, RuntimeError("boom on B")] + + await main() + + # Second print is the error response for task B. + error_output = json.loads(mock_print.call_args_list[1][0][0]) + assert error_output["status"] == "error" + assert error_output["request_id"] == "req-B" + + @pytest.mark.asyncio + @patch("sys.stdin") + @patch("builtins.print") + @patch("cpex.framework.isolated.worker.process_task") + async def test_main_error_before_parse_reports_unknown( + self, mock_process_task, mock_print, mock_stdin + ): + """A malformed line after a good task reports "unknown", not the prior id. + + The prior task set request_id to a real value; the per-iteration reset + ensures a subsequent JSON decode error does not inherit it. + """ + task_a = {"task_type": "info", "request_id": "req-A"} + mock_stdin.readline.side_effect = [ + json.dumps(task_a) + "\n", + "not-json\n", + "", # EOF + ] + + ok = MagicMock() + ok.model_dump.return_value = {"status": "success"} + mock_process_task.return_value = ok + + await main() + + error_output = json.loads(mock_print.call_args_list[1][0][0]) + assert error_output["status"] == "error" + assert error_output["request_id"] == "unknown" + # Made with Bob diff --git a/tests/unit/cpex/framework/test_utils.py b/tests/unit/cpex/framework/test_utils.py index 06e267b5..088933ef 100644 --- a/tests/unit/cpex/framework/test_utils.py +++ b/tests/unit/cpex/framework/test_utils.py @@ -18,7 +18,13 @@ ToolPostInvokePayload, ToolPreInvokePayload, ) -from cpex.framework.utils import import_module, matches, parse_class_name, payload_matches +from cpex.framework.utils import ( + import_module, + manifest_filename_for_class, + matches, + parse_class_name, + payload_matches, +) def test_server_ids(): @@ -125,6 +131,23 @@ def test_parse_class_name(): assert class_name == "MyClass" +def test_manifest_filename_for_class(): + """manifest_filename_for_class yields a unique, filesystem-safe name per class.""" + # Dotted class path is sanitized into a single safe filename. + assert manifest_filename_for_class("pkg.module.ClassName") == "pkg-module-ClassName.plugin-manifest.yaml" + + # Plugins sharing a package (same class_root) get distinct filenames — this + # is the collision the key change fixes. + assert manifest_filename_for_class("pkg.a.PluginA") != manifest_filename_for_class("pkg.b.PluginB") + + # Underscores and hyphens are preserved; other separators collapse to '-'. + assert manifest_filename_for_class("my_pkg.Cls") == "my_pkg-Cls.plugin-manifest.yaml" + + # Degenerate/empty input still yields a valid filename. + assert manifest_filename_for_class("") == "plugin.plugin-manifest.yaml" + assert manifest_filename_for_class("...") == "plugin.plugin-manifest.yaml" + + # ============================================================================ # Test payload_matches for prompt hooks # ============================================================================ diff --git a/tests/unit/cpex/tools/test_catalog.py b/tests/unit/cpex/tools/test_catalog.py index 557fc7a9..cf77928b 100644 --- a/tests/unit/cpex/tools/test_catalog.py +++ b/tests/unit/cpex/tools/test_catalog.py @@ -15,17 +15,17 @@ import tarfile import zipfile from pathlib import Path -from unittest import mock -from unittest.mock import MagicMock, Mock, patch, mock_open +from unittest.mock import MagicMock, Mock, patch # Third-Party import httpx import pytest import yaml +from cpex.framework.models import Monorepo, PluginManifest + # First-Party from cpex.tools.catalog import PluginCatalog -from cpex.framework.models import PluginManifest, Monorepo # Helper function to create test manifests @@ -40,7 +40,11 @@ def create_test_manifest(**kwargs): "tags": ["test"], "available_hooks": ["tools"], "default_config": {}, - "monorepo": Monorepo(package_source="https://github.com/org/repo#subdirectory=plugin", repo_url="https://github.com/org/repo", package_folder="plugin"), + "monorepo": Monorepo( + package_source="https://github.com/org/repo#subdirectory=plugin", + repo_url="https://github.com/org/repo", + package_folder="plugin", + ), } defaults.update(kwargs) return PluginManifest(**defaults) @@ -178,7 +182,7 @@ def test_save_manifest_content(self, tmp_path, mock_github_env): catalog.catalog_folder = str(tmp_path / "catalog") catalog_dir = tmp_path / "catalog" / "test_plugin" catalog_dir.mkdir(parents=True) - + manifest_yaml = """ name: test_plugin version: 1.0.0 @@ -191,10 +195,10 @@ def test_save_manifest_content(self, tmp_path, mock_github_env): """ repo_url = httpx.URL("https://github.com/org/repo") catalog.save_manifest_content(manifest_yaml, "test_plugin/plugin-manifest.yaml", repo_url) - + saved_file = tmp_path / "catalog" / "test_plugin" / "plugin-manifest.yaml" assert saved_file.exists() - + saved_data = yaml.safe_load(saved_file.read_text()) assert saved_data["monorepo"]["package_source"] == "https://github.com/org/repo#subdirectory=test_plugin" assert "tags" in saved_data @@ -207,7 +211,7 @@ def test_save_manifest_content_without_name(self, tmp_path, mock_github_env): catalog.catalog_folder = str(tmp_path / "catalog") catalog_dir = tmp_path / "catalog" / "test_plugin" catalog_dir.mkdir(parents=True) - + manifest_yaml = """ version: 1.0.0 kind: native @@ -219,10 +223,10 @@ def test_save_manifest_content_without_name(self, tmp_path, mock_github_env): """ repo_url = httpx.URL("https://github.com/org/repo") catalog.save_manifest_content(manifest_yaml, "test_plugin/plugin-manifest.yaml", repo_url) - + saved_file = tmp_path / "catalog" / "test_plugin" / "plugin-manifest.yaml" assert saved_file.exists() - + saved_data = yaml.safe_load(saved_file.read_text()) assert saved_data["name"] == "test_plugin" # Should be set from path @@ -232,7 +236,7 @@ def test_save_manifest_content_with_null_default_configs(self, tmp_path, mock_gi catalog.catalog_folder = str(tmp_path / "catalog") catalog_dir = tmp_path / "catalog" / "test_plugin" catalog_dir.mkdir(parents=True) - + manifest_yaml = """ name: test_plugin version: 1.0.0 @@ -244,10 +248,10 @@ def test_save_manifest_content_with_null_default_configs(self, tmp_path, mock_gi """ repo_url = httpx.URL("https://github.com/org/repo") catalog.save_manifest_content(manifest_yaml, "test_plugin/plugin-manifest.yaml", repo_url) - + saved_file = tmp_path / "catalog" / "test_plugin" / "plugin-manifest.yaml" assert saved_file.exists() - + saved_data = yaml.safe_load(saved_file.read_text()) assert saved_data["default_config"] == {} # Should be empty dict @@ -260,7 +264,7 @@ def test_download_contents_success(self, tmp_path, mock_github_env): with patch("cpex.tools.catalog.httpx.get") as mock_get: catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Mock the HTTP response manifest_content = "name: test\nversion: 1.0.0\nkind: native\ndescription: Test\nauthor: Test\navailable_hooks: [tools]\ndefault_config: {}" b64_content = base64.b64encode(manifest_content.encode()).decode() @@ -268,12 +272,12 @@ def test_download_contents_success(self, tmp_path, mock_github_env): mock_response.status_code = 200 mock_response.json.return_value = {"content": b64_content} mock_get.return_value = mock_response - + repo_url = httpx.URL("https://github.com/org/repo") # download_contents calls create_catalog_folder which creates the directory # then save_manifest_content writes the file catalog.download_contents("https://api.github.com/file", {}, "test_plugin/plugin-manifest.yaml", repo_url) - + assert (tmp_path / "catalog" / "test_plugin" / "plugin-manifest.yaml").exists() def test_download_contents_failure(self, tmp_path, mock_github_env): @@ -284,14 +288,14 @@ def test_download_contents_failure(self, tmp_path, mock_github_env): ): catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + mock_response = Mock() mock_response.status_code = 404 mock_get.return_value = mock_response - + repo_url = httpx.URL("https://github.com/org/repo") catalog.download_contents("https://api.github.com/file", {}, "test/plugin-manifest.yaml", repo_url) - + mock_logger.error.assert_called_once() @@ -321,7 +325,7 @@ def test_load_with_manifest_files(self, tmp_path, mock_github_env): """Test load with valid manifest files.""" catalog_dir = tmp_path / "catalog" / "test_plugin" catalog_dir.mkdir(parents=True) - + manifest_data = { "name": "test_plugin", "version": "1.0.0", @@ -334,11 +338,11 @@ def test_load_with_manifest_files(self, tmp_path, mock_github_env): } manifest_file = catalog_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") catalog.load() - + assert len(catalog.manifests) == 1 assert catalog.manifests[0].name == "test_plugin" @@ -347,14 +351,14 @@ def test_load_with_invalid_manifest(self, tmp_path, mock_github_env): with patch("cpex.tools.catalog.logger") as mock_logger: catalog_dir = tmp_path / "catalog" catalog_dir.mkdir() - + manifest_file = catalog_dir / "plugin-manifest.yaml" manifest_file.write_text("invalid: yaml: content:") - + catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") catalog.load() - + assert len(catalog.manifests) == 0 mock_logger.error.assert_called() @@ -408,7 +412,7 @@ def test_search_loads_manifests_if_empty(self, tmp_path, mock_github_env): """Test search loads manifests if catalog is empty.""" catalog_dir = tmp_path / "catalog" / "test_plugin" catalog_dir.mkdir(parents=True) - + manifest_data = { "name": "test_plugin", "version": "1.0.0", @@ -421,11 +425,11 @@ def test_search_loads_manifests_if_empty(self, tmp_path, mock_github_env): } manifest_file = catalog_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") result = catalog.search("test") - + assert result is not None assert len(result) == 1 @@ -452,7 +456,7 @@ def test_install_from_pypi_success(self, tmp_path, mock_github_env): } manifest_file = package_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", return_value=manifest_file), @@ -466,16 +470,16 @@ def test_install_from_pypi_success(self, tmp_path, mock_github_env): mock_dist.name = "test_package" mock_dist.files = None # No files attribute for non-isolated plugins mock_distributions.return_value = [mock_dist] - + # Setup mock spec for find_spec fallback mock_spec = Mock() mock_spec.origin = str(package_dir / "__init__.py") mock_find_spec.return_value = mock_spec - + catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") manifest, plugin_path = catalog.install_from_pypi("test_package") - + # Should call subprocess.run for non-isolated plugin mock_subprocess.assert_called_once() assert manifest.name == "test_package" @@ -485,7 +489,9 @@ def test_install_from_pypi_success(self, tmp_path, mock_github_env): def test_install_from_pypi_install_failure(self, mock_github_env): """Test installation failure from PyPI.""" - with patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", side_effect=RuntimeError("Download failed")): + with patch( + "cpex.tools.catalog.PluginCatalog._download_package_to_temp", side_effect=RuntimeError("Download failed") + ): catalog = PluginCatalog() with pytest.raises(RuntimeError, match="Download failed"): catalog.install_from_pypi("test_package") @@ -508,7 +514,7 @@ def test_install_from_pypi_package_not_found(self, tmp_path, mock_github_env): } manifest_file = package_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", return_value=manifest_file), @@ -526,10 +532,13 @@ def test_install_from_pypi_manifest_not_found(self, tmp_path, mock_github_env): """Test when manifest file is not found in package.""" extract_dir = tmp_path / "extracted" extract_dir.mkdir() - + with ( patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), - patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", side_effect=FileNotFoundError("plugin-manifest.yaml not found")), + patch( + "cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", + side_effect=FileNotFoundError("plugin-manifest.yaml not found"), + ), patch("shutil.rmtree"), ): catalog = PluginCatalog() @@ -545,7 +554,7 @@ def test_install_from_pypi_invalid_manifest(self, tmp_path, mock_github_env): package_dir.mkdir() manifest_file = package_dir / "plugin-manifest.yaml" manifest_file.write_text("invalid: yaml: content:") - + with ( patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", return_value=manifest_file), @@ -567,10 +576,10 @@ def test_install_folder_via_pip_success(self, tmp_path, mock_github_env): monorepo=Monorepo( package_source="https://github.com/org/repo#subdirectory=plugin", repo_url="https://github.com/org/repo", - package_folder="plugin" + package_folder="plugin", ) ) - + catalog = PluginCatalog() catalog.install_folder_via_pip(manifest) mock_subprocess.assert_called_once() @@ -578,7 +587,7 @@ def test_install_folder_via_pip_success(self, tmp_path, mock_github_env): def test_install_folder_via_pip_no_monorepo(self, mock_github_env): """Test installation fails when monorepo is None.""" manifest = create_test_manifest(monorepo=None) - + catalog = PluginCatalog() with pytest.raises(RuntimeError, match="PluginManifest.monorepo can not be None"): catalog.install_folder_via_pip(manifest) @@ -592,10 +601,10 @@ def test_install_folder_via_pip_subprocess_error(self, mock_github_env): monorepo=Monorepo( package_source="https://github.com/org/repo#subdirectory=plugin", repo_url="https://github.com/org/repo", - package_folder="plugin" + package_folder="plugin", ) ) - + catalog = PluginCatalog() with pytest.raises(RuntimeError, match="Failed to install"): catalog.install_folder_via_pip(manifest) @@ -608,15 +617,15 @@ def test_save_manifest(self, tmp_path, mock_github_env): """Test saving a manifest to the catalog.""" catalog_dir = tmp_path / "catalog" / "test_plugin" catalog_dir.mkdir(parents=True) - + manifest = create_test_manifest(name="test_plugin") catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") catalog.save_manifest(manifest, "test_plugin/plugin-manifest.yaml") - + saved_file = tmp_path / "catalog" / "test_plugin" / "plugin-manifest.yaml" assert saved_file.exists() - + saved_data = yaml.safe_load(saved_file.read_text()) assert saved_data["name"] == "test_plugin" @@ -627,7 +636,7 @@ class TestPluginCatalogDownloadFile: def test_download_file_success(self, mock_github_env): """Test successful file download.""" catalog = PluginCatalog() - + # Mock the GitHub repository and file content mock_repo = Mock() mock_file_content = Mock() @@ -635,10 +644,10 @@ def test_download_file_success(self, mock_github_env): mock_file_content.decoded_content = manifest_content.encode() mock_repo.get_contents.return_value = mock_file_content catalog.gh.get_repo = Mock(return_value=mock_repo) - + item = {"path": "test_plugin/plugin-manifest.yaml"} result = catalog.download_file("org/repo", item, {}, mock_repo) - + assert result == manifest_content def test_download_file_failure(self, mock_github_env): @@ -650,7 +659,7 @@ def test_download_file_failure(self, mock_github_env): mock_repo.get_contents.return_value = Exception("Not found") item = {"path": "test_plugin/plugin-manifest.yaml"} result = catalog.download_file("org/repo", item, {}, mock_repo) - + assert result is None mock_logger.error.assert_called_once() @@ -662,7 +671,7 @@ def test_find_and_save_plugin_manifest_success(self, tmp_path, mock_github_env): """Test successful finding and saving of plugin manifest.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Mock the search results mock_search_result = Mock() mock_content_file = Mock() @@ -670,12 +679,12 @@ def test_find_and_save_plugin_manifest_success(self, tmp_path, mock_github_env): mock_content_file.path = "test_plugin/plugin-manifest.yaml" mock_content_file.git_url = "https://api.github.com/repos/org/repo/git/blobs/abc123" mock_content_file.html_url = "https://github.com/org/repo/blob/main/test_plugin/plugin-manifest.yaml" - + mock_search_result.totalCount = 1 mock_search_result.__iter__ = Mock(return_value=iter([mock_content_file])) - + catalog.gh.search_code = Mock(return_value=mock_search_result) - + # Mock the repository and file content mock_repo = Mock() manifest_content = "name: test\nversion: 1.0.0\nkind: native\ndescription: Test\nauthor: Test\navailable_hooks: [tools]\ndefault_config: {}" @@ -683,10 +692,10 @@ def test_find_and_save_plugin_manifest_success(self, tmp_path, mock_github_env): mock_file_content.decoded_content = manifest_content.encode() mock_repo.get_contents.return_value = mock_file_content catalog.gh.get_repo = Mock(return_value=mock_repo) - + repo_url = httpx.URL("https://github.com/org/repo") catalog.find_and_save_plugin_manifest("test_plugin", "test_plugin", repo_url, {}, mock_repo) - + saved_file = tmp_path / "catalog" / "test_plugin" / "plugin-manifest.yaml" assert saved_file.exists() @@ -700,26 +709,21 @@ def test_update_catalog_with_pyproject_success(self, tmp_path, mock_github_env): catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") catalog.monorepos = ["https://github.com/org/repo"] - + # Mock search response for pyproject.toml files search_response = Mock() search_response.status_code = 200 search_response.json.return_value = { - "items": [ - { - "name": "pyproject.toml", - "path": "plugin1/pyproject.toml" - } - ] + "items": [{"name": "pyproject.toml", "path": "plugin1/pyproject.toml"}] } - + # Mock pyproject.toml content response pyproject_content = '[project]\nname = "test_plugin"' b64_pyproject = base64.b64encode(pyproject_content.encode()).decode() pyproject_response = Mock() pyproject_response.status_code = 200 pyproject_response.json.return_value = {"content": b64_pyproject} - + # Mock search response for manifest manifest_search_response = Mock() manifest_search_response.status_code = 200 @@ -728,22 +732,24 @@ def test_update_catalog_with_pyproject_success(self, tmp_path, mock_github_env): { "name": "plugin-manifest.yaml", "path": "plugin1/plugin-manifest.yaml", - "git_url": "https://api.github.com/repos/org/repo/git/blobs/abc123" + "git_url": "https://api.github.com/repos/org/repo/git/blobs/abc123", } ] } - + # Mock manifest content response - manifest_content = "name: test\nversion: 1.0.0\nkind: native\ndescription: Test\nauthor: Test\navailable_hooks: [tools]" + manifest_content = ( + "name: test\nversion: 1.0.0\nkind: native\ndescription: Test\nauthor: Test\navailable_hooks: [tools]" + ) b64_manifest = base64.b64encode(manifest_content.encode()).decode() manifest_response = Mock() manifest_response.status_code = 200 manifest_response.json.return_value = {"content": b64_manifest} - + mock_get.side_effect = [search_response, pyproject_response, manifest_search_response, manifest_response] - + catalog.update_catalog_with_pyproject() - + assert (tmp_path / "catalog").exists() @@ -783,7 +789,7 @@ def test_install_from_pypi_with_version_constraint(self, tmp_path, mock_github_e } manifest_file = package_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", return_value=manifest_file), @@ -796,16 +802,16 @@ def test_install_from_pypi_with_version_constraint(self, tmp_path, mock_github_e mock_dist.name = "test_package" mock_dist.files = None # No files attribute for non-isolated plugins mock_distributions.return_value = [mock_dist] - + # Setup mock spec for find_spec fallback mock_spec = Mock() mock_spec.origin = str(package_dir / "__init__.py") mock_find_spec.return_value = mock_spec - + catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") manifest, plugin_path = catalog.install_from_pypi("test_package", ">=1.0.0") - + mock_subprocess.assert_called_once() assert manifest.name == "test_package" assert manifest.package_info is not None @@ -829,7 +835,7 @@ def test_install_from_pypi_with_default_configs(self, tmp_path, mock_github_env) } manifest_file = package_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", return_value=manifest_file), @@ -842,16 +848,16 @@ def test_install_from_pypi_with_default_configs(self, tmp_path, mock_github_env) mock_dist.name = "test_package" mock_dist.files = None # No files attribute for non-isolated plugins mock_distributions.return_value = [mock_dist] - + # Setup mock spec for find_spec fallback mock_spec = Mock() mock_spec.origin = str(package_dir / "__init__.py") mock_find_spec.return_value = mock_spec - + catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") manifest, plugin_path = catalog.install_from_pypi("test_package") - + assert manifest.default_config == {"key": "value"} def test_install_from_pypi_with_existing_package_info(self, tmp_path, mock_github_env): @@ -869,14 +875,11 @@ def test_install_from_pypi_with_existing_package_info(self, tmp_path, mock_githu "tags": ["test"], "available_hooks": ["tools"], "default_config": {}, - "package_info": { - "pypi_package": "old_name", - "version_constraint": ">=0.1.0" - } + "package_info": {"pypi_package": "old_name", "version_constraint": ">=0.1.0"}, } manifest_file = package_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", return_value=manifest_file), @@ -889,16 +892,16 @@ def test_install_from_pypi_with_existing_package_info(self, tmp_path, mock_githu mock_dist.name = "test_package" mock_dist.files = None # No files attribute for non-isolated plugins mock_distributions.return_value = [mock_dist] - + # Setup mock spec for find_spec fallback mock_spec = Mock() mock_spec.origin = str(package_dir / "__init__.py") mock_find_spec.return_value = mock_spec - + catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") manifest, plugin_path = catalog.install_from_pypi("test_package", ">=2.0.0") - + assert manifest.package_info is not None assert manifest.package_info.pypi_package == "test_package" assert manifest.package_info.version_constraint == ">=2.0.0" @@ -921,7 +924,7 @@ def test_install_from_pypi_with_null_default_configs_in_manifest(self, tmp_path, } manifest_file = package_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", return_value=manifest_file), @@ -934,16 +937,16 @@ def test_install_from_pypi_with_null_default_configs_in_manifest(self, tmp_path, mock_dist.name = "test_package" mock_dist.files = None # No files attribute for non-isolated plugins mock_distributions.return_value = [mock_dist] - + # Setup mock spec for find_spec fallback mock_spec = Mock() mock_spec.origin = str(package_dir / "__init__.py") mock_find_spec.return_value = mock_spec - + catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") manifest, plugin_path = catalog.install_from_pypi("test_package") - + # default_config should be empty dict when default_configs is None assert manifest.default_config == {} @@ -951,7 +954,6 @@ def test_install_from_pypi_with_null_default_configs_in_manifest(self, tmp_path, # Made with Bob - class TestPluginCatalogProcessPyproject: """Tests for _process_pyproject helper method.""" @@ -959,18 +961,18 @@ def test_process_pyproject_with_download_failure(self, tmp_path, mock_github_env """Test _process_pyproject when download fails.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Mock the repository to raise exception mock_repo = Mock() mock_repo.get_contents = Mock(side_effect=Exception("Download failed")) - + item = Mock() item.name = "pyproject.toml" item.path = "plugin1/pyproject.toml" - + repo_url = httpx.URL("https://github.com/org/repo") headers = {} - + # Should raise exception with pytest.raises(Exception, match="Download failed"): catalog._process_pyproject(mock_repo, item, repo_url, headers) @@ -987,7 +989,7 @@ def test_update_catalog_with_pyproject_no_token(self, tmp_path, mock_github_env) catalog = PluginCatalog() catalog.github_token = None result = catalog.update_catalog_with_pyproject() - + assert result is True mock_logger.error.assert_called_with("No GitHub token set") @@ -999,12 +1001,12 @@ def test_update_catalog_with_pyproject_repo_access_error(self, tmp_path, mock_gi catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") catalog.monorepos = ["https://github.com/org/repo"] - + # Mock get_repo to raise exception catalog.gh.get_repo = Mock(side_effect=Exception("Access denied")) - + result = catalog.update_catalog_with_pyproject() - + assert result is False mock_logger.error.assert_called() @@ -1014,14 +1016,14 @@ def test_update_catalog_with_pyproject_search_error(self, tmp_path, mock_github_ catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") catalog.monorepos = ["https://github.com/org/repo"] - + # Mock successful get_repo but failing search mock_repo = Mock() catalog.gh.get_repo = Mock(return_value=mock_repo) catalog.gh.search_code = Mock(side_effect=Exception("Search failed")) - + result = catalog.update_catalog_with_pyproject() - + assert result is False mock_logger.error.assert_called() @@ -1033,12 +1035,12 @@ def test_search_github_code_exception(self, mock_github_env): """Test _search_github_code when exception occurs.""" with patch("cpex.tools.catalog.logger") as mock_logger: catalog = PluginCatalog() - + # Mock search_code to raise exception catalog.gh.search_code = Mock(side_effect=Exception("Search error")) - + result = catalog._search_github_code("org/repo", "plugins", {}) - + assert result is None mock_logger.error.assert_called() @@ -1051,18 +1053,16 @@ def test_process_manifest_item_not_yaml(self, tmp_path, mock_github_env): with patch("cpex.tools.catalog.logger") as mock_logger: catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - - item = { - "name": "README.md", - "path": "plugin1/README.md", - "git_url": "https://api.github.com/file" - } - + + item = {"name": "README.md", "path": "plugin1/README.md", "git_url": "https://api.github.com/file"} + repo_url = httpx.URL("https://github.com/org/repo") relpath = tmp_path / "catalog" / "plugin1" / "plugin-manifest.yaml" mock_repo = Mock() - result = catalog._process_manifest_item(item, "plugin1", "plugin1", repo_url, {}, relpath, "org/repo", gh_repo=mock_repo) - + result = catalog._process_manifest_item( + item, "plugin1", "plugin1", repo_url, {}, relpath, "org/repo", gh_repo=mock_repo + ) + assert result is False mock_logger.warning.assert_called() @@ -1071,21 +1071,23 @@ def test_process_manifest_item_download_failure(self, tmp_path, mock_github_env) with patch("cpex.tools.catalog.logger") as mock_logger: catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Mock download_file to return None catalog.download_file = Mock(return_value=None) - + item = { "name": "plugin-manifest.yaml", "path": "plugin1/plugin-manifest.yaml", - "git_url": "https://api.github.com/file" + "git_url": "https://api.github.com/file", } mock_repo = Mock() repo_url = httpx.URL("https://github.com/org/repo") relpath = tmp_path / "catalog" / "plugin1" / "plugin-manifest.yaml" - - result = catalog._process_manifest_item(item, "plugin1", "plugin1", repo_url, {}, relpath, "org/repo", gh_repo=mock_repo) - + + result = catalog._process_manifest_item( + item, "plugin1", "plugin1", repo_url, {}, relpath, "org/repo", gh_repo=mock_repo + ) + assert result is False mock_logger.error.assert_called() @@ -1097,13 +1099,13 @@ def test_find_and_save_plugin_manifest_search_returns_none(self, tmp_path, mock_ """Test find_and_save_plugin_manifest when search returns None.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Mock _search_github_code to return None catalog._search_github_code = Mock(return_value=None) mock_repo = Mock() repo_url = httpx.URL("https://github.com/org/repo") result = catalog.find_and_save_plugin_manifest("plugin1", "plugin1", repo_url, {}, mock_repo) - + assert result is None @@ -1206,7 +1208,7 @@ def test_load_manifest_file_not_found(self, tmp_path, mock_github_env): """Test _load_manifest_file when file doesn't exist.""" catalog = PluginCatalog() manifest_path = tmp_path / "nonexistent" / "plugin-manifest.yaml" - + with pytest.raises(FileNotFoundError, match="plugin-manifest.yaml not found"): catalog._load_manifest_file(manifest_path) @@ -1215,7 +1217,7 @@ def test_load_manifest_file_invalid_yaml(self, tmp_path, mock_github_env): catalog = PluginCatalog() manifest_path = tmp_path / "plugin-manifest.yaml" manifest_path.write_text("invalid: yaml: content:") - + with pytest.raises(RuntimeError, match="Failed to parse manifest YAML"): catalog._load_manifest_file(manifest_path) @@ -1224,7 +1226,7 @@ def test_load_manifest_file_not_dict(self, tmp_path, mock_github_env): catalog = PluginCatalog() manifest_path = tmp_path / "plugin-manifest.yaml" manifest_path.write_text("- item1\n- item2") - + with pytest.raises(RuntimeError, match="Invalid manifest format"): catalog._load_manifest_file(manifest_path) @@ -1235,10 +1237,10 @@ class TestPluginCatalogNormalizeManifestData: def test_normalize_manifest_data_validation_error(self, mock_github_env): """Test _normalize_manifest_data with validation error.""" catalog = PluginCatalog() - + # Invalid manifest data (missing required fields) manifest_data = {"name": "test"} - + with pytest.raises(RuntimeError, match="Failed to validate manifest"): catalog._normalize_manifest_data(manifest_data, "test_package", None) @@ -1250,9 +1252,9 @@ def test_persist_manifest_error(self, tmp_path, mock_github_env): """Test _persist_manifest when save fails.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "nonexistent" / "catalog") - + manifest = create_test_manifest() - + # Make directory read-only to cause save failure with patch("cpex.tools.catalog.PluginCatalog.save_manifest", side_effect=Exception("Save failed")): with pytest.raises(RuntimeError, match="Failed to save manifest"): @@ -1267,7 +1269,7 @@ def test_install_package_with_version_constraint(self, mock_github_env): with patch("cpex.tools.catalog.subprocess.run") as mock_subprocess: catalog = PluginCatalog() catalog._install_package("test_package", ">=1.0.0") - + mock_subprocess.assert_called_once() call_args = mock_subprocess.call_args[0][0] assert "test_package>=1.0.0" in " ".join(call_args) @@ -1280,14 +1282,14 @@ def test_download_file_with_exception_message(self, mock_github_env): """Test download_file logs proper error message.""" with patch("cpex.tools.catalog.logger") as mock_logger: catalog = PluginCatalog() - + # Mock to raise exception catalog.gh.get_repo = Mock(side_effect=Exception("API error")) mock_repo = Mock() mock_repo.get_contents = Mock(side_effect=Exception("API error")) item = {"path": "test/file.yaml"} result = catalog.download_file("org/repo", item, {}, gh_repo=mock_repo) - + assert result is None # Check that error was logged with the item path assert mock_logger.error.called @@ -1299,22 +1301,22 @@ class TestPluginCatalogSearchGithubCodeWithNullMember: def test_search_github_code_with_null_member(self, mock_github_env): """Test _search_github_code when member is None.""" catalog = PluginCatalog() - + # Mock the search results mock_search_results = MagicMock() mock_search_results.totalCount = 1 - + mock_content_file = MagicMock() mock_content_file.name = "plugin-manifest.yaml" mock_content_file.path = "plugin-manifest.yaml" mock_content_file.git_url = "https://api.github.com/repos/org/repo/git/blobs/abc123" mock_content_file.html_url = "https://github.com/org/repo/blob/main/plugin-manifest.yaml" - + mock_search_results.__iter__ = Mock(return_value=iter([mock_content_file])) - - with patch.object(catalog.gh, 'search_code', return_value=mock_search_results): + + with patch.object(catalog.gh, "search_code", return_value=mock_search_results): result = catalog._search_github_code("org/repo", None, {}) - + assert result is not None assert len(result) == 1 assert result[0]["name"] == "plugin-manifest.yaml" @@ -1326,7 +1328,7 @@ class TestPluginCatalogTransformManifestDataWithNullMember: def test_transform_manifest_data_with_null_member(self, mock_github_env): """Test _transform_manifest_data when member is None.""" catalog = PluginCatalog() - + manifest_content = { "version": "1.0.0", "kind": "native", @@ -1334,10 +1336,10 @@ def test_transform_manifest_data_with_null_member(self, mock_github_env): "author": "Test Author", "available_hooks": ["tools"], } - + repo_url = httpx.URL("https://github.com/org/repo") result = catalog._transform_manifest_data(manifest_content, "test_plugin", None, repo_url) - + assert result["name"] == "test_plugin" assert result["monorepo"]["package_source"] == "https://github.com/org/repo" assert result["monorepo"]["package_folder"] == "" @@ -1349,7 +1351,7 @@ class TestPluginCatalogDownloadMonorepoFolderToTemp: def test_download_monorepo_folder_success(self, tmp_path, mock_github_env): """Test successful download of monorepo folder.""" catalog = PluginCatalog() - + # Create a mock tarball mock_tarball = tmp_path / "package.tar.gz" with tarfile.open(mock_tarball, "w:gz") as tar: @@ -1357,74 +1359,70 @@ def test_download_monorepo_folder_success(self, tmp_path, mock_github_env): temp_file = tmp_path / "test_file.txt" temp_file.write_text("test content") tar.add(temp_file, arcname="test_file.txt") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - - with patch('pathlib.Path.glob') as mock_glob: + + with patch("pathlib.Path.glob") as mock_glob: mock_glob.return_value = [mock_tarball] - + result = catalog._download_monorepo_folder_to_temp( - "git+https://github.com/org/repo#subdirectory=plugin", - "test_plugin" + "git+https://github.com/org/repo#subdirectory=plugin", "test_plugin" ) - + assert result.exists() assert result.name == "extracted" def test_download_monorepo_folder_no_files(self, tmp_path, mock_github_env): """Test error when no files are downloaded.""" catalog = PluginCatalog() - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - - with patch('pathlib.Path.glob') as mock_glob: + + with patch("pathlib.Path.glob") as mock_glob: mock_glob.return_value = [] - + with pytest.raises(RuntimeError) as exc_info: catalog._download_monorepo_folder_to_temp( - "git+https://github.com/org/repo#subdirectory=plugin", - "test_plugin" + "git+https://github.com/org/repo#subdirectory=plugin", "test_plugin" ) - + assert "No files downloaded" in str(exc_info.value) def test_download_monorepo_folder_unsupported_format(self, tmp_path, mock_github_env): """Test error with unsupported package format.""" catalog = PluginCatalog() - + # Create a mock file with unsupported extension mock_file = tmp_path / "package.unknown" mock_file.write_text("test") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - - with patch('pathlib.Path.glob') as mock_glob: + + with patch("pathlib.Path.glob") as mock_glob: mock_glob.return_value = [mock_file] - + with pytest.raises(RuntimeError) as exc_info: catalog._download_monorepo_folder_to_temp( - "git+https://github.com/org/repo#subdirectory=plugin", - "test_plugin" + "git+https://github.com/org/repo#subdirectory=plugin", "test_plugin" ) - + assert "Unsupported package format" in str(exc_info.value) def test_download_monorepo_folder_subprocess_error(self, mock_github_env): """Test subprocess error handling.""" catalog = PluginCatalog() - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.side_effect = subprocess.CalledProcessError(1, "pip", stderr="Download failed") - + with pytest.raises(RuntimeError) as exc_info: catalog._download_monorepo_folder_to_temp( - "git+https://github.com/org/repo#subdirectory=plugin", - "test_plugin" + "git+https://github.com/org/repo#subdirectory=plugin", "test_plugin" ) - + assert "Failed to download" in str(exc_info.value) @@ -1434,23 +1432,23 @@ class TestPluginCatalogDownloadPackageToTemp: def test_download_package_with_test_pypi(self, tmp_path, mock_github_env): """Test downloading from test.pypi.org.""" catalog = PluginCatalog() - + # Create a mock wheel file mock_wheel = tmp_path / "package-1.0.0-py3-none-any.whl" with zipfile.ZipFile(mock_wheel, "w") as zf: zf.writestr("test_file.txt", "test content") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - - with patch('pathlib.Path.glob') as mock_glob: + + with patch("pathlib.Path.glob") as mock_glob: mock_glob.return_value = [mock_wheel] - + result = catalog._download_package_to_temp("test_plugin", None, use_test=True) - + assert result.exists() assert result.name == "extracted" - + # Verify test.pypi.org was used call_args = mock_run.call_args[0][0] assert "--index-url" in call_args @@ -1459,34 +1457,34 @@ def test_download_package_with_test_pypi(self, tmp_path, mock_github_env): def test_download_package_no_files_downloaded(self, mock_github_env): """Test error when no files are downloaded.""" catalog = PluginCatalog() - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - - with patch('pathlib.Path.glob') as mock_glob: + + with patch("pathlib.Path.glob") as mock_glob: mock_glob.return_value = [] - + with pytest.raises(RuntimeError) as exc_info: catalog._download_package_to_temp("test_plugin", None) - + assert "No files downloaded" in str(exc_info.value) def test_download_package_unsupported_format(self, tmp_path, mock_github_env): """Test error with unsupported package format.""" catalog = PluginCatalog() - + mock_file = tmp_path / "package.exe" mock_file.write_text("test") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - - with patch('pathlib.Path.glob') as mock_glob: + + with patch("pathlib.Path.glob") as mock_glob: mock_glob.return_value = [mock_file] - + with pytest.raises(RuntimeError) as exc_info: catalog._download_package_to_temp("test_plugin", None) - + assert "Unsupported package format" in str(exc_info.value) @@ -1496,13 +1494,13 @@ class TestPluginCatalogFindManifestInExtractedPackage: def test_find_manifest_not_found(self, tmp_path, mock_github_env): """Test FileNotFoundError when manifest is not found.""" catalog = PluginCatalog() - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() - + with pytest.raises(FileNotFoundError) as exc_info: catalog._find_manifest_in_extracted_package(extract_dir, "test_plugin") - + assert "plugin-manifest.yaml not found" in str(exc_info.value) @@ -1512,15 +1510,15 @@ class TestPluginCatalogUninstallPackage: def test_uninstall_package_success_native(self, mock_github_env): """Test successful package uninstallation for native plugin.""" catalog = PluginCatalog() - + # Create a native plugin manifest manifest = create_test_manifest(kind="native") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - + result = catalog.uninstall_package("test_plugin", manifest) - + assert result is True mock_run.assert_called_once() call_args = mock_run.call_args[0][0] @@ -1535,10 +1533,10 @@ def test_uninstall_package_success_isolated_venv(self, tmp_path, mock_github_env """Test successful package uninstallation for isolated_venv plugin.""" catalog = PluginCatalog() catalog.plugin_folder = str(tmp_path / "plugins") - + # Create an isolated_venv plugin manifest manifest = create_test_manifest(kind="isolated_venv") - + # Create mock venv structure plugin_path = tmp_path / "plugins" / "test_plugin" plugin_path.mkdir(parents=True) @@ -1547,20 +1545,20 @@ def test_uninstall_package_success_isolated_venv(self, tmp_path, mock_github_env venv_bin.mkdir(parents=True) venv_python = venv_bin / "python" venv_python.touch() - + # Mock the IsolatedVenvPlugin mock_isolated_plugin = MagicMock() mock_isolated_plugin.plugin_path = plugin_path - + with ( - patch('subprocess.run') as mock_run, - patch('cpex.framework.isolated.client.IsolatedVenvPlugin', return_value=mock_isolated_plugin), - patch.object(catalog, '_get_venv_python_executable', return_value=str(venv_python)), + patch("subprocess.run") as mock_run, + patch("cpex.framework.isolated.client.IsolatedVenvPlugin", return_value=mock_isolated_plugin), + patch.object(catalog, "_get_venv_python_executable", return_value=str(venv_python)), ): mock_run.return_value = MagicMock(returncode=0) - + result = catalog.uninstall_package("test_plugin", manifest) - + assert result is True mock_run.assert_called_once() call_args = mock_run.call_args[0][0] @@ -1575,26 +1573,26 @@ def test_uninstall_package_subprocess_error(self, mock_github_env): """Test subprocess error during uninstallation.""" catalog = PluginCatalog() manifest = create_test_manifest(kind="native") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.side_effect = subprocess.CalledProcessError(1, "pip", stderr="Uninstall failed") - + with pytest.raises(RuntimeError) as exc_info: catalog.uninstall_package("test_plugin", manifest) - + assert "Failed to uninstall" in str(exc_info.value) def test_uninstall_package_unexpected_error(self, mock_github_env): """Test unexpected error during uninstallation.""" catalog = PluginCatalog() manifest = create_test_manifest(kind="native") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.side_effect = Exception("Unexpected error") - + with pytest.raises(RuntimeError) as exc_info: catalog.uninstall_package("test_plugin", manifest) - + assert "Unexpected error uninstalling" in str(exc_info.value) def test_uninstall_package_isolated_venv_error(self, tmp_path, mock_github_env): @@ -1602,7 +1600,7 @@ def test_uninstall_package_isolated_venv_error(self, tmp_path, mock_github_env): catalog = PluginCatalog() catalog.plugin_folder = str(tmp_path / "plugins") manifest = create_test_manifest(kind="isolated_venv") - + # Create mock venv structure plugin_path = tmp_path / "plugins" / "test_plugin" plugin_path.mkdir(parents=True) @@ -1611,21 +1609,21 @@ def test_uninstall_package_isolated_venv_error(self, tmp_path, mock_github_env): venv_bin.mkdir(parents=True) venv_python = venv_bin / "python" venv_python.touch() - + # Mock the IsolatedVenvPlugin mock_isolated_plugin = MagicMock() mock_isolated_plugin.plugin_path = plugin_path - + with ( - patch('subprocess.run') as mock_run, - patch('cpex.framework.isolated.client.IsolatedVenvPlugin', return_value=mock_isolated_plugin), - patch.object(catalog, '_get_venv_python_executable', return_value=str(venv_python)), + patch("subprocess.run") as mock_run, + patch("cpex.framework.isolated.client.IsolatedVenvPlugin", return_value=mock_isolated_plugin), + patch.object(catalog, "_get_venv_python_executable", return_value=str(venv_python)), ): mock_run.side_effect = subprocess.CalledProcessError(1, "pip", stderr="Uninstall failed") - + with pytest.raises(RuntimeError) as exc_info: catalog.uninstall_package("test_plugin", manifest) - + assert "Failed to uninstall" in str(exc_info.value) @@ -1635,22 +1633,26 @@ class TestPluginCatalogInstallFolderViaPipIsolated: def test_install_folder_via_pip_isolated_venv(self, tmp_path, mock_github_env): """Test installing an isolated_venv plugin from monorepo.""" catalog = PluginCatalog() - + manifest = create_test_manifest(kind="isolated_venv") - + # Mock the download and initialization - with patch.object(catalog, '_download_monorepo_folder_to_temp') as mock_download: + with patch.object(catalog, "_download_monorepo_folder_to_temp") as mock_download: mock_download.return_value = tmp_path / "package" - - with patch.object(catalog, '_initialize_isolated_venv') as mock_init: + + with ( + patch.object(catalog, "_initialize_isolated_venv") as mock_init, + patch.object(catalog, "_install_package_into_venv") as mock_venv_install, + ): mock_init.return_value = tmp_path / "venv" - + result = catalog.install_folder_via_pip(manifest) - + assert result == tmp_path / "venv" mock_download.assert_called_once() mock_init.assert_called_once() - + # Plugin package is installed into the venv from the monorepo source (U4). + mock_venv_install.assert_called_once() class TestPluginCatalogProcessPyprojectExtended: @@ -1659,12 +1661,12 @@ class TestPluginCatalogProcessPyprojectExtended: def test_process_pyproject_with_member_none(self, mock_github_env): """Test _process_pyproject when member is None (root directory).""" catalog = PluginCatalog() - + mock_repo = MagicMock() mock_item = MagicMock() mock_item.path = "pyproject.toml" mock_item.name = "pyproject.toml" - + # Mock file content pyproject_content = """ [project] @@ -1672,18 +1674,18 @@ def test_process_pyproject_with_member_none(self, mock_github_env): version = "1.0.0" """ mock_file_content = MagicMock() - mock_file_content.decoded_content = pyproject_content.encode('utf-8') + mock_file_content.decoded_content = pyproject_content.encode("utf-8") mock_repo.get_contents.return_value = mock_file_content - + repo_url = httpx.URL("https://github.com/org/repo") - - with patch.object(catalog, 'find_and_save_plugin_manifest') as mock_find: + + with patch.object(catalog, "find_and_save_plugin_manifest") as mock_find: catalog._process_pyproject(mock_repo, mock_item, repo_url, {}) - + # Verify find_and_save_plugin_manifest was called with member=None mock_find.assert_called_once() call_args = mock_find.call_args - assert call_args[1]['member'] is None + assert call_args[1]["member"] is None class TestPluginCatalogInstallFolderViaPipNonIsolated: @@ -1692,17 +1694,17 @@ class TestPluginCatalogInstallFolderViaPipNonIsolated: def test_install_folder_via_pip_non_isolated(self, mock_github_env): """Test installing a non-isolated plugin from monorepo.""" catalog = PluginCatalog() - + manifest = create_test_manifest(kind="native") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - + result = catalog.install_folder_via_pip(manifest) - + # For non-isolated plugins, should return None assert result is None - + # Verify pip install was called mock_run.assert_called_once() call_args = mock_run.call_args[0][0] @@ -1716,12 +1718,12 @@ class TestPluginCatalogInstallPackageEdgeCases: def test_install_package_with_null_version_constraint(self, mock_github_env): """Test installing package with None version constraint.""" catalog = PluginCatalog() - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - + catalog._install_package("test_plugin", None, use_test=False) - + mock_run.assert_called_once() call_args = mock_run.call_args[0][0] assert "test_plugin" in call_args @@ -1730,13 +1732,13 @@ def test_install_package_with_null_version_constraint(self, mock_github_env): def test_install_package_unexpected_error(self, mock_github_env): """Test unexpected error during package installation.""" catalog = PluginCatalog() - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.side_effect = Exception("Unexpected error") - + with pytest.raises(RuntimeError) as exc_info: catalog._install_package("test_plugin", None) - + assert "Unexpected error installing" in str(exc_info.value) @@ -1746,21 +1748,21 @@ class TestPluginCatalogDownloadPackageEdgeCases: def test_download_package_with_version_constraint(self, tmp_path, mock_github_env): """Test downloading package with version constraint.""" catalog = PluginCatalog() - + mock_wheel = tmp_path / "package-1.0.0-py3-none-any.whl" with zipfile.ZipFile(mock_wheel, "w") as zf: zf.writestr("test_file.txt", "test content") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - - with patch('pathlib.Path.glob') as mock_glob: + + with patch("pathlib.Path.glob") as mock_glob: mock_glob.return_value = [mock_wheel] - + result = catalog._download_package_to_temp("test_plugin", ">=1.0.0", use_test=False) - + assert result.exists() - + # Verify version constraint was included call_args = mock_run.call_args[0][0] assert any("test_plugin>=1.0.0" in str(arg) for arg in call_args) @@ -1768,23 +1770,22 @@ def test_download_package_with_version_constraint(self, tmp_path, mock_github_en def test_download_monorepo_zip_format(self, tmp_path, mock_github_env): """Test downloading monorepo with zip format.""" catalog = PluginCatalog() - + # Create a mock zip file mock_zip = tmp_path / "package.zip" with zipfile.ZipFile(mock_zip, "w") as zf: zf.writestr("test_file.txt", "test content") - - with patch('subprocess.run') as mock_run: + + with patch("subprocess.run") as mock_run: mock_run.return_value = MagicMock(returncode=0) - - with patch('pathlib.Path.glob') as mock_glob: + + with patch("pathlib.Path.glob") as mock_glob: mock_glob.return_value = [mock_zip] - + result = catalog._download_monorepo_folder_to_temp( - "git+https://github.com/org/repo#subdirectory=plugin", - "test_plugin" + "git+https://github.com/org/repo#subdirectory=plugin", "test_plugin" ) - + assert result.exists() assert result.name == "extracted" @@ -1795,12 +1796,12 @@ class TestPluginCatalogFindOperations: def test_find_case_insensitive(self, tmp_path, mock_github_env): """Test that find is case-insensitive.""" catalog = PluginCatalog() - + # Create a manifest with uppercase name manifest_dir = tmp_path / "catalog" / "TEST_PLUGIN" manifest_dir.mkdir(parents=True) manifest_file = manifest_dir / "plugin-manifest.yaml" - + manifest_data = { "name": "TEST_PLUGIN", "version": "1.0.0", @@ -1812,13 +1813,13 @@ def test_find_case_insensitive(self, tmp_path, mock_github_env): "default_config": {}, } manifest_file.write_text(yaml.safe_dump(manifest_data)) - + catalog.catalog_folder = str(tmp_path / "catalog") catalog.load() - + # Search with lowercase should find it result = catalog.find("test_plugin") - + assert result is not None assert result.name == "TEST_PLUGIN" @@ -1829,13 +1830,13 @@ class TestPluginCatalogInstallFromPypiIsolated: def test_install_from_pypi_isolated_venv(self, tmp_path, mock_github_env): """Test installing an isolated_venv plugin from PyPI.""" catalog = PluginCatalog() - + # Create mock extracted package with manifest extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "test_plugin" plugin_dir.mkdir() - + manifest_file = plugin_dir / "plugin-manifest.yaml" manifest_data = { "name": "test_plugin", @@ -1847,20 +1848,70 @@ def test_install_from_pypi_isolated_venv(self, tmp_path, mock_github_env): "default_config": {"requirements_file": "requirements.txt"}, } manifest_file.write_text(yaml.safe_dump(manifest_data)) - - with patch.object(catalog, '_download_package_to_temp') as mock_download: + + with patch.object(catalog, "_download_package_to_temp") as mock_download: mock_download.return_value = extract_dir - - with patch.object(catalog, '_initialize_isolated_venv') as mock_init: + + with ( + patch.object(catalog, "_initialize_isolated_venv") as mock_init, + patch.object(catalog, "_install_package_into_venv") as mock_venv_install, + ): mock_init.return_value = tmp_path / "venv" - - with patch.object(catalog, '_persist_manifest'): + + with patch.object(catalog, "_persist_manifest"): manifest, plugin_path = catalog.install_from_pypi("test_plugin") - + assert manifest.kind == "isolated_venv" assert plugin_path == tmp_path / "venv" mock_init.assert_called_once() + # Plugin package is installed into the venv from PyPI (U4). + mock_venv_install.assert_called_once() + + def test_install_from_test_pypi_isolated_venv_uses_extra_index(self, tmp_path, mock_github_env): + """test-pypi isolated_venv install pairs test.pypi with real PyPI (#5). + + The isolated venv is fresh and empty, so transitive deps (including cpex + itself) must resolve. Installing with --index-url alone would require + every dep on test.pypi; an --extra-index-url to real PyPI is required. + """ + catalog = PluginCatalog() + + extract_dir = tmp_path / "extracted" + extract_dir.mkdir() + plugin_dir = extract_dir / "test_plugin" + plugin_dir.mkdir() + + manifest_file = plugin_dir / "plugin-manifest.yaml" + manifest_data = { + "name": "test_plugin", + "version": "1.0.0", + "kind": "isolated_venv", + "description": "Test isolated plugin", + "author": "Test Author", + "available_hooks": ["tools"], + "default_config": {"requirements_file": "requirements.txt"}, + } + manifest_file.write_text(yaml.safe_dump(manifest_data)) + + with patch.object(catalog, "_download_package_to_temp") as mock_download: + mock_download.return_value = extract_dir + with ( + patch.object(catalog, "_initialize_isolated_venv") as mock_init, + patch.object(catalog, "_install_package_into_venv") as mock_venv_install, + patch.object(catalog, "_persist_manifest"), + ): + mock_init.return_value = tmp_path / "venv" + catalog.install_from_pypi("test_plugin", ">=1.0.0", use_pytest=True) + + mock_venv_install.assert_called_once() + pip_args = mock_venv_install.call_args[0][1] + assert "--index-url" in pip_args + assert "https://test.pypi.org/simple/" in pip_args + assert "--extra-index-url" in pip_args + assert "https://pypi.org/simple/" in pip_args + # The versioned target is still present. + assert "test_plugin>=1.0.0" in pip_args class TestPluginCatalogFindRequirementsInExtractedPackage: @@ -1869,7 +1920,7 @@ class TestPluginCatalogFindRequirementsInExtractedPackage: def test_find_requirements_success(self, tmp_path, mock_github_env): """Test successful finding of requirements file.""" catalog = PluginCatalog() - + # Create a mock extracted package directory with requirements.txt extract_dir = tmp_path / "extracted" extract_dir.mkdir() @@ -1877,122 +1928,108 @@ def test_find_requirements_success(self, tmp_path, mock_github_env): plugin_dir.mkdir() requirements_file = plugin_dir / "requirements.txt" requirements_file.write_text("pytest>=7.0.0\n") - + # Find the requirements file - result = catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", "requirements.txt" - ) - + result = catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", "requirements.txt") + assert result == requirements_file assert result.exists() def test_find_requirements_not_found(self, tmp_path, mock_github_env): """Test FileNotFoundError when requirements file doesn't exist.""" catalog = PluginCatalog() - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() - + with pytest.raises(FileNotFoundError) as exc_info: - catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", "requirements.txt" - ) - + catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", "requirements.txt") + assert "requirements file requirements.txt not found" in str(exc_info.value) assert "my_plugin" in str(exc_info.value) def test_find_requirements_path_traversal_parent_directory(self, tmp_path, mock_github_env): """Test that path traversal with ../ is blocked.""" catalog = PluginCatalog() - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() - + # Try to access parent directory with pytest.raises(ValueError) as exc_info: - catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", "../../../etc/passwd" - ) - + catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", "../../../etc/passwd") + assert "path traversal attempts are not allowed" in str(exc_info.value) def test_find_requirements_path_traversal_absolute_path(self, tmp_path, mock_github_env): """Test that absolute paths are blocked.""" catalog = PluginCatalog() - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() - + # Try to use absolute path with pytest.raises(ValueError) as exc_info: - catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", "/etc/passwd" - ) - + catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", "/etc/passwd") + assert "path traversal attempts are not allowed" in str(exc_info.value) def test_find_requirements_path_traversal_mixed_separators(self, tmp_path, mock_github_env): """Test that mixed path separators are blocked.""" catalog = PluginCatalog() - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() - + # Try to use backslashes (Windows-style) in suspicious way with pytest.raises(ValueError) as exc_info: - catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", "..\\..\\etc\\passwd" - ) - + catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", "..\\..\\etc\\passwd") + assert "path traversal attempts are not allowed" in str(exc_info.value) def test_find_requirements_path_traversal_encoded(self, tmp_path, mock_github_env): """Test that URL-encoded path traversal attempts are blocked.""" catalog = PluginCatalog() - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() - + # Try various encoded forms malicious_paths = [ "..%2F..%2Fetc%2Fpasswd", "..%5c..%5cetc%5cpasswd", ] - + for malicious_path in malicious_paths: with pytest.raises(ValueError) as exc_info: - catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", malicious_path - ) - + catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", malicious_path) + assert "path traversal" in str(exc_info.value).lower() or "suspicious" in str(exc_info.value).lower() def test_find_requirements_defense_in_depth(self, tmp_path, mock_github_env): """Test defense-in-depth check that file is within extract_dir.""" catalog = PluginCatalog() - + # Create extract directory extract_dir = tmp_path / "extracted" extract_dir.mkdir() - + # Create a file outside the extract directory outside_dir = tmp_path / "outside" outside_dir.mkdir() outside_file = outside_dir / "requirements.txt" outside_file.write_text("malicious\n") - + # Create a symlink inside extract_dir pointing outside # (This tests the defense-in-depth check) try: symlink_path = extract_dir / "requirements.txt" symlink_path.symlink_to(outside_file) - + # The rglob should find it, but the defense-in-depth check should catch it with pytest.raises(ValueError) as exc_info: - catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", "requirements.txt" - ) - + catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", "requirements.txt") + assert "outside the package directory" in str(exc_info.value) except OSError: # Skip test if symlinks aren't supported (e.g., Windows without admin) @@ -2001,7 +2038,7 @@ def test_find_requirements_defense_in_depth(self, tmp_path, mock_github_env): def test_find_requirements_nested_directory(self, tmp_path, mock_github_env): """Test finding requirements file in nested directory structure.""" catalog = PluginCatalog() - + # Create nested directory structure extract_dir = tmp_path / "extracted" extract_dir.mkdir() @@ -2009,58 +2046,52 @@ def test_find_requirements_nested_directory(self, tmp_path, mock_github_env): nested_dir.mkdir(parents=True) requirements_file = nested_dir / "requirements.txt" requirements_file.write_text("pytest>=7.0.0\n") - + # Should find the file in nested structure - result = catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", "requirements.txt" - ) - + result = catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", "requirements.txt") + assert result == requirements_file assert result.exists() def test_find_requirements_multiple_files_returns_first(self, tmp_path, mock_github_env): """Test that when multiple matching files exist, the first one is returned.""" catalog = PluginCatalog() - + # Create multiple requirements files extract_dir = tmp_path / "extracted" extract_dir.mkdir() - + dir1 = extract_dir / "dir1" dir1.mkdir() req1 = dir1 / "requirements.txt" req1.write_text("first\n") - + dir2 = extract_dir / "dir2" dir2.mkdir() req2 = dir2 / "requirements.txt" req2.write_text("second\n") - + # Should return one of them (first found by rglob) - result = catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", "requirements.txt" - ) - + result = catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", "requirements.txt") + assert result in [req1, req2] assert result.exists() def test_find_requirements_custom_filename(self, tmp_path, mock_github_env): """Test finding a custom requirements filename.""" catalog = PluginCatalog() - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "my_plugin" plugin_dir.mkdir() - + # Use a custom requirements filename custom_req = plugin_dir / "requirements-dev.txt" custom_req.write_text("pytest>=7.0.0\n") - - result = catalog._find_requirements_in_extracted_package( - extract_dir, "my_plugin", "requirements-dev.txt" - ) - + + result = catalog._find_requirements_in_extracted_package(extract_dir, "my_plugin", "requirements-dev.txt") + assert result == custom_req assert result.exists() @@ -2072,18 +2103,18 @@ def test_find_and_load_versions_json_non_isolated_success(self, tmp_path, mock_g """Test _find_and_load_versions_json for non-isolated plugin with versions.json.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create a plugin path with versions.json plugin_path = tmp_path / "plugin_package" plugin_path.mkdir() versions_json = plugin_path / "versions.json" versions_data = {"versions": [{"version": "1.0.0", "date": "2024-01-01"}]} versions_json.write_text(json.dumps(versions_data)) - + manifest = create_test_manifest(name="test_plugin", kind="native") - + catalog._find_and_load_versions_json(manifest, plugin_path, "test_plugin") - + # Check that versions.json was saved to catalog catalog_versions = Path(catalog.catalog_folder) / "test_plugin" / "versions.json" assert catalog_versions.exists() @@ -2095,15 +2126,15 @@ def test_find_and_load_versions_json_non_isolated_no_file(self, tmp_path, mock_g with patch("cpex.tools.catalog.logger") as mock_logger: catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create a plugin path without versions.json plugin_path = tmp_path / "plugin_package" plugin_path.mkdir() - + manifest = create_test_manifest(name="test_plugin", kind="native") - + catalog._find_and_load_versions_json(manifest, plugin_path, "test_plugin") - + # Check that no versions.json was saved to catalog catalog_versions = Path(catalog.catalog_folder) / "test_plugin" / "versions.json" assert not catalog_versions.exists() @@ -2113,20 +2144,20 @@ def test_find_and_load_versions_json_isolated_success(self, tmp_path, mock_githu """Test _find_and_load_versions_json for isolated_venv plugin.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create a venv path venv_path = tmp_path / ".venv" venv_path.mkdir() - + # Mock the subprocess call to find package path mock_package_path = tmp_path / "venv_package" mock_package_path.mkdir() versions_json = mock_package_path / "versions.json" versions_data = {"versions": [{"version": "2.0.0", "date": "2024-02-01"}]} versions_json.write_text(json.dumps(versions_data)) - + manifest = create_test_manifest(name="test_plugin", kind="isolated_venv") - + with ( patch.object(catalog, "_get_venv_python_executable", return_value="/fake/python"), patch("cpex.tools.catalog.subprocess.run") as mock_run, @@ -2136,9 +2167,9 @@ def test_find_and_load_versions_json_isolated_success(self, tmp_path, mock_githu mock_result.stdout = str(mock_package_path) mock_result.stderr = "" mock_run.return_value = mock_result - + catalog._find_and_load_versions_json(manifest, venv_path, "test_plugin") - + # Check that versions.json was saved to catalog catalog_versions = Path(catalog.catalog_folder) / "test_plugin" / "versions.json" assert catalog_versions.exists() @@ -2150,21 +2181,21 @@ def test_find_and_load_versions_json_isolated_subprocess_failure(self, tmp_path, with patch("cpex.tools.catalog.logger") as mock_logger: catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + venv_path = tmp_path / ".venv" venv_path.mkdir() - + manifest = create_test_manifest(name="test_plugin", kind="isolated_venv") - + with patch("cpex.tools.catalog.subprocess.run") as mock_run: mock_result = Mock() mock_result.returncode = 1 mock_result.stdout = "" mock_result.stderr = "NOT_FOUND" mock_run.return_value = mock_result - + catalog._find_and_load_versions_json(manifest, venv_path, "test_plugin") - + # Check that no versions.json was saved catalog_versions = Path(catalog.catalog_folder) / "test_plugin" / "versions.json" assert not catalog_versions.exists() @@ -2174,12 +2205,12 @@ def test_find_and_load_versions_json_none_plugin_path(self, tmp_path, mock_githu """Test _find_and_load_versions_json with None plugin_path.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + manifest = create_test_manifest(name="test_plugin", kind="native") - + # Should handle None gracefully catalog._find_and_load_versions_json(manifest, None, "test_plugin") - + catalog_versions = Path(catalog.catalog_folder) / "test_plugin" / "versions.json" assert not catalog_versions.exists() @@ -2188,18 +2219,18 @@ def test_find_and_load_versions_json_exception_handling(self, tmp_path, mock_git with patch("cpex.tools.catalog.logger") as mock_logger: catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + plugin_path = tmp_path / "plugin_package" plugin_path.mkdir() - + manifest = create_test_manifest(name="test_plugin", kind="native") - + # Create a versions.json that will cause an error when reading versions_json = plugin_path / "versions.json" versions_json.write_text("invalid json {{{") - + catalog._find_and_load_versions_json(manifest, plugin_path, "test_plugin") - + mock_logger.warning.assert_called() @@ -2209,14 +2240,14 @@ class TestPluginCatalogGetVenvPythonExecutable: def test_get_venv_python_executable_unix(self, tmp_path, mock_github_env): """Test _get_venv_python_executable on Unix-like systems.""" catalog = PluginCatalog() - + venv_path = tmp_path / ".venv" venv_path.mkdir() bin_dir = venv_path / "bin" bin_dir.mkdir() python_exe = bin_dir / "python" python_exe.touch() - + with patch("sys.platform", "linux"): result = catalog._get_venv_python_executable(venv_path) assert result == str(python_exe) @@ -2224,14 +2255,14 @@ def test_get_venv_python_executable_unix(self, tmp_path, mock_github_env): def test_get_venv_python_executable_windows(self, tmp_path, mock_github_env): """Test _get_venv_python_executable on Windows.""" catalog = PluginCatalog() - + venv_path = tmp_path / ".venv" venv_path.mkdir() scripts_dir = venv_path / "Scripts" scripts_dir.mkdir() python_exe = scripts_dir / "python.exe" python_exe.touch() - + with patch("sys.platform", "win32"): result = catalog._get_venv_python_executable(venv_path) assert result == str(python_exe) @@ -2239,10 +2270,10 @@ def test_get_venv_python_executable_windows(self, tmp_path, mock_github_env): def test_get_venv_python_executable_not_found(self, tmp_path, mock_github_env): """Test _get_venv_python_executable when executable doesn't exist.""" catalog = PluginCatalog() - + venv_path = tmp_path / ".venv" venv_path.mkdir() - + with pytest.raises(FileNotFoundError, match="Python executable not found"): catalog._get_venv_python_executable(venv_path) @@ -2254,13 +2285,13 @@ def test_install_from_pypi_calls_find_and_load_versions_json(self, tmp_path, moc """Test that install_from_pypi calls _find_and_load_versions_json.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create temporary package structure temp_extract = tmp_path / "temp_extract" temp_extract.mkdir() package_dir = temp_extract / "test_plugin" package_dir.mkdir() - + manifest_path = package_dir / "plugin-manifest.yaml" manifest_data = { "name": "test_plugin", @@ -2270,10 +2301,10 @@ def test_install_from_pypi_calls_find_and_load_versions_json(self, tmp_path, moc "author": "Test", "tags": ["test"], "available_hooks": ["tools"], - "default_config": {} + "default_config": {}, } manifest_path.write_text(yaml.dump(manifest_data)) - + with ( patch.object(catalog, "_download_package_to_temp", return_value=temp_extract), patch.object(catalog, "_install_package"), @@ -2282,7 +2313,7 @@ def test_install_from_pypi_calls_find_and_load_versions_json(self, tmp_path, moc patch.object(catalog, "update_plugin_version_registry"), ): manifest, plugin_path = catalog.install_from_pypi("test_plugin") - + # Verify _find_and_load_versions_json was called mock_find_versions.assert_called_once() call_args = mock_find_versions.call_args @@ -2301,12 +2332,12 @@ def test_install_from_local_manifest_in_root(self, tmp_path, mock_github_env): """Test installing from local source with manifest in root directory.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create source directory with pyproject and manifest in root source_dir = tmp_path / "my_plugin" source_dir.mkdir() (source_dir / "pyproject.toml").write_text('[project]\nname = "my_plugin"\nversion = "1.0.0"\n') - + manifest_data = { "name": "my_plugin", "version": "1.0.0", @@ -2319,7 +2350,7 @@ def test_install_from_local_manifest_in_root(self, tmp_path, mock_github_env): } manifest_file = source_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.find_package_path", return_value=source_dir), @@ -2328,7 +2359,7 @@ def test_install_from_local_manifest_in_root(self, tmp_path, mock_github_env): patch.object(catalog, "update_plugin_version_registry"), ): manifest, plugin_path = catalog.install_from_local(source_dir) - + # Verify subprocess was called with pip install -e mock_subprocess.assert_called_once() call_args = mock_subprocess.call_args[0][0] @@ -2337,7 +2368,7 @@ def test_install_from_local_manifest_in_root(self, tmp_path, mock_github_env): assert "install" in call_args assert "-e" in call_args assert str(source_dir) in call_args - + assert manifest.name == "my_plugin" assert manifest.kind == "native" assert plugin_path == source_dir @@ -2346,14 +2377,14 @@ def test_install_from_local_manifest_in_subdirectory(self, tmp_path, mock_github """Test installing from local source with manifest in subdirectory.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create source directory with pyproject and manifest in subdirectory source_dir = tmp_path / "my_plugin_project" source_dir.mkdir() plugin_subdir = source_dir / "my_plugin" plugin_subdir.mkdir() (plugin_subdir / "pyproject.toml").write_text('[project]\nname = "my_plugin"\nversion = "1.0.0"\n') - + manifest_data = { "name": "my_plugin", "version": "1.0.0", @@ -2366,7 +2397,7 @@ def test_install_from_local_manifest_in_subdirectory(self, tmp_path, mock_github } manifest_file = plugin_subdir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.find_package_path", return_value=source_dir), @@ -2375,18 +2406,18 @@ def test_install_from_local_manifest_in_subdirectory(self, tmp_path, mock_github patch.object(catalog, "update_plugin_version_registry"), ): manifest, plugin_path = catalog.install_from_local(source_dir) - + assert manifest.name == "my_plugin" mock_subprocess.assert_called_once() def test_install_from_local_manifest_not_found(self, tmp_path, mock_github_env): """Test error when manifest is not found in source or subdirectories.""" catalog = PluginCatalog() - + # Create source directory without pyproject or manifest source_dir = tmp_path / "my_plugin" source_dir.mkdir() - + with pytest.raises(FileNotFoundError, match="pyproject.toml not found"): catalog.install_from_local(source_dir) @@ -2395,12 +2426,12 @@ def test_install_from_local_isolated_venv(self, tmp_path, mock_github_env): catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") catalog.plugin_folder = str(tmp_path / "plugins") - + # Create source directory with isolated_venv pyproject and manifest source_dir = tmp_path / "my_isolated_plugin" source_dir.mkdir() (source_dir / "pyproject.toml").write_text('[project]\nname = "my_isolated_plugin"\nversion = "1.0.0"\n') - + manifest_data = { "name": "my_isolated_plugin", "version": "1.0.0", @@ -2413,14 +2444,14 @@ def test_install_from_local_isolated_venv(self, tmp_path, mock_github_env): } manifest_file = source_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + # Mock IsolatedVenvPlugin mock_isolated_plugin = Mock() mock_isolated_plugin.plugin_path = tmp_path / "plugins" / "my_isolated_plugin" mock_isolated_plugin.plugin_path.mkdir(parents=True, exist_ok=True) venv_path = mock_isolated_plugin.plugin_path / ".venv" venv_path.mkdir(parents=True, exist_ok=True) - + # Create mock venv python executable if sys.platform == "win32": python_exe = venv_path / "Scripts" / "python.exe" @@ -2428,7 +2459,7 @@ def test_install_from_local_isolated_venv(self, tmp_path, mock_github_env): python_exe = venv_path / "bin" / "python" python_exe.parent.mkdir(parents=True, exist_ok=True) python_exe.touch() - + with ( patch("cpex.framework.isolated.client.IsolatedVenvPlugin", return_value=mock_isolated_plugin), patch("asyncio.run"), @@ -2438,7 +2469,7 @@ def test_install_from_local_isolated_venv(self, tmp_path, mock_github_env): patch.object(catalog, "update_plugin_version_registry"), ): manifest, plugin_path = catalog.install_from_local(source_dir) - + # Verify subprocess was called with venv python mock_subprocess.assert_called_once() call_args = mock_subprocess.call_args[0][0] @@ -2448,7 +2479,7 @@ def test_install_from_local_isolated_venv(self, tmp_path, mock_github_env): assert "install" in call_args assert "-e" in call_args assert str(source_dir) in call_args - + assert manifest.name == "my_isolated_plugin" assert manifest.kind == "isolated_venv" assert plugin_path == mock_isolated_plugin.plugin_path @@ -2457,12 +2488,12 @@ def test_install_from_local_subprocess_error(self, tmp_path, mock_github_env): """Test error handling when pip install fails.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create source directory with pyproject and manifest source_dir = tmp_path / "my_plugin" source_dir.mkdir() (source_dir / "pyproject.toml").write_text('[project]\nname = "my_plugin"\nversion = "1.0.0"\n') - + manifest_data = { "name": "my_plugin", "version": "1.0.0", @@ -2475,27 +2506,27 @@ def test_install_from_local_subprocess_error(self, tmp_path, mock_github_env): } manifest_file = source_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with patch("cpex.tools.catalog.subprocess.run") as mock_subprocess: mock_subprocess.side_effect = subprocess.CalledProcessError( 1, ["pip", "install"], stderr="Installation failed" ) - + with pytest.raises(RuntimeError, match="Failed to install plugin from"): catalog.install_from_local(source_dir) def test_install_from_local_invalid_manifest(self, tmp_path, mock_github_env): """Test error handling when manifest is invalid.""" catalog = PluginCatalog() - + # Create source directory with pyproject and invalid manifest source_dir = tmp_path / "my_plugin" source_dir.mkdir() (source_dir / "pyproject.toml").write_text('[project]\nname = "my_plugin"\nversion = "1.0.0"\n') - + manifest_file = source_dir / "plugin-manifest.yaml" manifest_file.write_text("invalid: yaml: content:") - + with pytest.raises(RuntimeError, match="Failed to parse manifest YAML"): catalog.install_from_local(source_dir) @@ -2503,12 +2534,12 @@ def test_install_from_local_calls_persist_and_registry(self, tmp_path, mock_gith """Test that install_from_local calls persist_manifest and update_plugin_version_registry.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create source directory with pyproject and manifest source_dir = tmp_path / "my_plugin" source_dir.mkdir() (source_dir / "pyproject.toml").write_text('[project]\nname = "my_plugin"\nversion = "1.0.0"\n') - + manifest_data = { "name": "my_plugin", "version": "1.0.0", @@ -2521,7 +2552,7 @@ def test_install_from_local_calls_persist_and_registry(self, tmp_path, mock_gith } manifest_file = source_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.subprocess.run"), patch("cpex.tools.catalog.find_package_path", return_value=source_dir), @@ -2530,12 +2561,12 @@ def test_install_from_local_calls_persist_and_registry(self, tmp_path, mock_gith patch.object(catalog, "update_plugin_version_registry") as mock_registry, ): manifest, plugin_path = catalog.install_from_local(source_dir) - + # Verify all post-install steps were called mock_persist.assert_called_once() mock_versions.assert_called_once() mock_registry.assert_called_once() - + # Verify the manifest was passed correctly persist_call_args = mock_persist.call_args[0] assert persist_call_args[0].name == "my_plugin" @@ -2545,12 +2576,12 @@ def test_install_from_local_isolated_venv_initialization_error(self, tmp_path, m catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") catalog.plugin_folder = str(tmp_path / "plugins") - + # Create source directory with isolated_venv pyproject and manifest source_dir = tmp_path / "my_isolated_plugin" source_dir.mkdir() (source_dir / "pyproject.toml").write_text('[project]\nname = "my_isolated_plugin"\nversion = "1.0.0"\n') - + manifest_data = { "name": "my_isolated_plugin", "version": "1.0.0", @@ -2563,13 +2594,13 @@ def test_install_from_local_isolated_venv_initialization_error(self, tmp_path, m } manifest_file = source_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.framework.isolated.client.IsolatedVenvPlugin") as mock_plugin_class, patch("asyncio.run") as mock_asyncio_run, ): mock_asyncio_run.side_effect = Exception("Venv initialization failed") - + with pytest.raises(RuntimeError, match="Failed to install isolated_venv plugin"): catalog.install_from_local(source_dir) @@ -2577,12 +2608,12 @@ def test_install_from_local_fallback_to_source_path(self, tmp_path, mock_github_ """Test that source path is used as fallback when find_package_path returns None.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create source directory with pyproject and manifest source_dir = tmp_path / "my_plugin" source_dir.mkdir() (source_dir / "pyproject.toml").write_text('[project]\nname = "my_plugin"\nversion = "1.0.0"\n') - + manifest_data = { "name": "my_plugin", "version": "1.0.0", @@ -2595,7 +2626,7 @@ def test_install_from_local_fallback_to_source_path(self, tmp_path, mock_github_ } manifest_file = source_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + with ( patch("cpex.tools.catalog.subprocess.run"), patch.object(catalog, "_persist_manifest"), @@ -2603,7 +2634,7 @@ def test_install_from_local_fallback_to_source_path(self, tmp_path, mock_github_ patch.object(catalog, "update_plugin_version_registry"), ): manifest, plugin_path = catalog.install_from_local(source_dir) - + # Non-isolated installs now derive plugin_path from manifest location assert plugin_path == source_dir @@ -2611,12 +2642,12 @@ def test_install_from_local_with_versions_json(self, tmp_path, mock_github_env): """Test that versions.json is found and loaded correctly.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create source directory with pyproject and manifest source_dir = tmp_path / "my_plugin" source_dir.mkdir() (source_dir / "pyproject.toml").write_text('[project]\nname = "my_plugin"\nversion = "1.0.0"\n') - + manifest_data = { "name": "my_plugin", "version": "1.0.0", @@ -2629,11 +2660,11 @@ def test_install_from_local_with_versions_json(self, tmp_path, mock_github_env): } manifest_file = source_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + # Create a different path that versions.json returns actual_path = tmp_path / "actual_plugin_path" actual_path.mkdir() - + with ( patch("cpex.tools.catalog.subprocess.run"), patch("cpex.tools.catalog.find_package_path", return_value=source_dir), @@ -2642,12 +2673,11 @@ def test_install_from_local_with_versions_json(self, tmp_path, mock_github_env): patch.object(catalog, "update_plugin_version_registry"), ): manifest, plugin_path = catalog.install_from_local(source_dir) - + # Should return the actual path from versions.json assert plugin_path == actual_path - class TestPluginCatalogInstallFromGit: """Tests for PluginCatalog.install_from_git method.""" @@ -2655,13 +2685,13 @@ def test_install_from_git_success_https(self, tmp_path, mock_github_env): """Test successful installation from Git using HTTPS URL.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create mock extracted package with manifest extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "test_plugin" plugin_dir.mkdir() - + manifest_data = { "name": "test_plugin", "version": "1.0.0", @@ -2674,12 +2704,12 @@ def test_install_from_git_success_https(self, tmp_path, mock_github_env): } manifest_file = plugin_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + # Create a mock archive archive_path = tmp_path / "test_plugin-1.0.0.tar.gz" with tarfile.open(archive_path, "w:gz") as tar: tar.add(plugin_dir, arcname="test_plugin") - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), @@ -2695,12 +2725,12 @@ def mock_run(*args, **kwargs): # Simulate pip download creating the archive pass return Mock(returncode=0) - + mock_subprocess.side_effect = mock_run - + url = "test_plugin @ git+https://github.com/example/test_plugin.git" manifest, plugin_path = catalog.install_from_git(url) - + assert manifest.name == "test_plugin" assert manifest.kind == "native" assert plugin_path == plugin_dir @@ -2711,12 +2741,12 @@ def test_install_from_git_success_ssh(self, tmp_path, mock_github_env): """Test successful installation from Git using SSH URL.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "test_plugin" plugin_dir.mkdir() - + manifest_data = { "name": "test_plugin", "version": "1.0.0", @@ -2729,11 +2759,11 @@ def test_install_from_git_success_ssh(self, tmp_path, mock_github_env): } manifest_file = plugin_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + archive_path = tmp_path / "test_plugin-1.0.0.tar.gz" with tarfile.open(archive_path, "w:gz") as tar: tar.add(plugin_dir, arcname="test_plugin") - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), @@ -2744,11 +2774,11 @@ def test_install_from_git_success_ssh(self, tmp_path, mock_github_env): patch("shutil.rmtree"), ): mock_subprocess.return_value = Mock(returncode=0) - + # Use git@ format which is the standard SSH format url = "test_plugin @ git+git@github.com:example/test_plugin.git" manifest, plugin_path = catalog.install_from_git(url) - + assert manifest.name == "test_plugin" assert plugin_path == plugin_dir @@ -2756,12 +2786,12 @@ def test_install_from_git_with_branch(self, tmp_path, mock_github_env): """Test installation from Git with specific branch.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "test_plugin" plugin_dir.mkdir() - + manifest_data = { "name": "test_plugin", "version": "1.0.0", @@ -2774,11 +2804,11 @@ def test_install_from_git_with_branch(self, tmp_path, mock_github_env): } manifest_file = plugin_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + archive_path = tmp_path / "test_plugin-1.0.0.tar.gz" with tarfile.open(archive_path, "w:gz") as tar: tar.add(plugin_dir, arcname="test_plugin") - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), @@ -2789,10 +2819,10 @@ def test_install_from_git_with_branch(self, tmp_path, mock_github_env): patch("shutil.rmtree"), ): mock_subprocess.return_value = Mock(returncode=0) - + url = "test_plugin @ git+https://github.com/example/test_plugin.git@master" manifest, plugin_path = catalog.install_from_git(url) - + assert manifest.name == "test_plugin" # Verify that the branch was included in the pip install command install_call = [call for call in mock_subprocess.call_args_list if "install" in str(call)] @@ -2802,12 +2832,12 @@ def test_install_from_git_isolated_venv(self, tmp_path, mock_github_env): """Test installation of isolated_venv plugin from Git.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "test_plugin" plugin_dir.mkdir() - + manifest_data = { "name": "test_plugin", "version": "1.0.0", @@ -2820,22 +2850,22 @@ def test_install_from_git_isolated_venv(self, tmp_path, mock_github_env): } manifest_file = plugin_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + # Create requirements file requirements_file = plugin_dir / "requirements.txt" requirements_file.write_text("pytest>=7.0.0\n") - + archive_path = tmp_path / "test_plugin-1.0.0.tar.gz" with tarfile.open(archive_path, "w:gz") as tar: tar.add(plugin_dir, arcname="test_plugin") tar.add(requirements_file, arcname="test_plugin/requirements.txt") - + venv_path = tmp_path / "venv_path" venv_path.mkdir() venv_bin = venv_path / "venv" / "bin" venv_bin.mkdir(parents=True) venv_python = venv_bin / "python" - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), @@ -2847,10 +2877,10 @@ def test_install_from_git_isolated_venv(self, tmp_path, mock_github_env): patch("shutil.rmtree"), ): mock_subprocess.return_value = Mock(returncode=0) - + url = "test_plugin @ git+https://github.com/example/test_plugin.git" manifest, plugin_path = catalog.install_from_git(url) - + assert manifest.kind == "isolated_venv" assert plugin_path == venv_path # Should call subprocess twice: download and install into isolated venv @@ -2863,35 +2893,35 @@ def test_install_from_git_isolated_venv(self, tmp_path, mock_github_env): def test_install_from_git_invalid_url_format(self, mock_github_env): """Test error when URL format is invalid (missing @).""" catalog = PluginCatalog() - + with pytest.raises(ValueError) as exc_info: catalog.install_from_git("test_plugin") - + assert "Invalid Git URL format" in str(exc_info.value) assert "Expected format" in str(exc_info.value) def test_install_from_git_missing_git_prefix(self, mock_github_env): """Test error when git+ prefix is missing.""" catalog = PluginCatalog() - + with pytest.raises(ValueError) as exc_info: catalog.install_from_git("test_plugin @ https://github.com/example/test_plugin.git") - + assert "Git URL must start with 'git+'" in str(exc_info.value) def test_install_from_git_invalid_git_url(self, mock_github_env): """Test error when Git URL is invalid.""" catalog = PluginCatalog() - + with pytest.raises(ValueError) as exc_info: catalog.install_from_git("test_plugin @ git+invalid://not-a-valid-url") - + assert "Invalid Git repository URL" in str(exc_info.value) def test_install_from_git_download_failure(self, tmp_path, mock_github_env): """Test error when pip download fails.""" catalog = PluginCatalog() - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), @@ -2900,53 +2930,53 @@ def test_install_from_git_download_failure(self, tmp_path, mock_github_env): mock_subprocess.side_effect = subprocess.CalledProcessError( 1, ["pip", "download"], stderr="Download failed" ) - + with pytest.raises(RuntimeError) as exc_info: catalog.install_from_git("test_plugin @ git+https://github.com/example/test_plugin.git") - + assert "Failed to install test_plugin from Git" in str(exc_info.value) def test_install_from_git_no_archive_found(self, tmp_path, mock_github_env): """Test error when no archive is found after download.""" catalog = PluginCatalog() - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), patch("shutil.rmtree"), ): mock_subprocess.return_value = Mock(returncode=0) - + with pytest.raises(RuntimeError) as exc_info: catalog.install_from_git("test_plugin @ git+https://github.com/example/test_plugin.git") - + assert "No package archive found" in str(exc_info.value) def test_install_from_git_manifest_not_found(self, tmp_path, mock_github_env): """Test error when manifest is not found in package.""" catalog = PluginCatalog() - + # Create archive without manifest extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "test_plugin" plugin_dir.mkdir() - + archive_path = tmp_path / "test_plugin-1.0.0.tar.gz" with tarfile.open(archive_path, "w:gz") as tar: tar.add(plugin_dir, arcname="test_plugin") - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), patch("shutil.rmtree"), ): mock_subprocess.return_value = Mock(returncode=0) - + # The method wraps FileNotFoundError in RuntimeError with pytest.raises(RuntimeError) as exc_info: catalog.install_from_git("test_plugin @ git+https://github.com/example/test_plugin.git") - + assert "Unexpected error installing test_plugin from Git" in str(exc_info.value) assert "plugin-manifest.yaml not found" in str(exc_info.value) @@ -2954,12 +2984,12 @@ def test_install_from_git_install_failure(self, tmp_path, mock_github_env): """Test error when pip install fails.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "test_plugin" plugin_dir.mkdir() - + manifest_data = { "name": "test_plugin", "version": "1.0.0", @@ -2972,11 +3002,11 @@ def test_install_from_git_install_failure(self, tmp_path, mock_github_env): } manifest_file = plugin_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + archive_path = tmp_path / "test_plugin-1.0.0.tar.gz" with tarfile.open(archive_path, "w:gz") as tar: tar.add(plugin_dir, arcname="test_plugin") - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), @@ -2987,16 +3017,16 @@ def test_install_from_git_install_failure(self, tmp_path, mock_github_env): Mock(returncode=0), # download succeeds subprocess.CalledProcessError(1, ["pip", "install"], stderr="Install failed"), # install fails ] - + with pytest.raises(RuntimeError) as exc_info: catalog.install_from_git("test_plugin @ git+https://github.com/example/test_plugin.git") - + assert "Failed to install test_plugin from Git" in str(exc_info.value) def test_install_from_git_cleanup_on_error(self, tmp_path, mock_github_env): """Test that temporary directory is cleaned up even on error.""" catalog = PluginCatalog() - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), @@ -3005,10 +3035,10 @@ def test_install_from_git_cleanup_on_error(self, tmp_path, mock_github_env): mock_subprocess.side_effect = subprocess.CalledProcessError( 1, ["pip", "download"], stderr="Download failed" ) - + with pytest.raises(RuntimeError): catalog.install_from_git("test_plugin @ git+https://github.com/example/test_plugin.git") - + # Verify cleanup was called mock_rmtree.assert_called_once() assert str(tmp_path) in str(mock_rmtree.call_args) @@ -3017,12 +3047,12 @@ def test_install_from_git_with_zip_archive(self, tmp_path, mock_github_env): """Test installation from Git with zip archive.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "test_plugin" plugin_dir.mkdir() - + manifest_data = { "name": "test_plugin", "version": "1.0.0", @@ -3035,12 +3065,12 @@ def test_install_from_git_with_zip_archive(self, tmp_path, mock_github_env): } manifest_file = plugin_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + # Create a zip archive archive_path = tmp_path / "test_plugin-1.0.0.zip" with zipfile.ZipFile(archive_path, "w") as zipf: zipf.write(manifest_file, arcname="test_plugin/plugin-manifest.yaml") - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), @@ -3051,22 +3081,22 @@ def test_install_from_git_with_zip_archive(self, tmp_path, mock_github_env): patch("shutil.rmtree"), ): mock_subprocess.return_value = Mock(returncode=0) - + url = "test_plugin @ git+https://github.com/example/test_plugin.git" manifest, plugin_path = catalog.install_from_git(url) - + assert manifest.name == "test_plugin" def test_install_from_git_with_wheel(self, tmp_path, mock_github_env): """Test installation from Git with wheel archive.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + extract_dir = tmp_path / "extracted" extract_dir.mkdir() plugin_dir = extract_dir / "test_plugin" plugin_dir.mkdir() - + manifest_data = { "name": "test_plugin", "version": "1.0.0", @@ -3079,12 +3109,12 @@ def test_install_from_git_with_wheel(self, tmp_path, mock_github_env): } manifest_file = plugin_dir / "plugin-manifest.yaml" manifest_file.write_text(yaml.safe_dump(manifest_data)) - + # Create a wheel archive (which is a zip file) archive_path = tmp_path / "test_plugin-1.0.0-py3-none-any.whl" with zipfile.ZipFile(archive_path, "w") as zipf: zipf.write(manifest_file, arcname="test_plugin/plugin-manifest.yaml") - + with ( patch("cpex.tools.catalog.subprocess.run") as mock_subprocess, patch("cpex.tools.catalog.tempfile.mkdtemp", return_value=str(tmp_path)), @@ -3095,10 +3125,10 @@ def test_install_from_git_with_wheel(self, tmp_path, mock_github_env): patch("shutil.rmtree"), ): mock_subprocess.return_value = Mock(returncode=0) - + url = "test_plugin @ git+https://github.com/example/test_plugin.git" manifest, plugin_path = catalog.install_from_git(url) - + assert manifest.name == "test_plugin" @@ -3106,6 +3136,7 @@ def test_install_from_git_with_wheel(self, tmp_path, mock_github_env): # _extract_package_archive — path traversal guards # --------------------------------------------------------------------------- + class TestExtractPackageArchivePathTraversal: """Verify that _extract_package_archive rejects archives with unsafe member paths.""" @@ -3121,6 +3152,7 @@ def catalog(self): def test_tar_traversal_rejected(self, catalog, tmp_path): """A tar member whose path escapes extract_dir raises and writes nothing.""" import io + archive = tmp_path / "evil.tar.gz" with tarfile.open(archive, "w:gz") as tf: data = b"pwned" @@ -3139,6 +3171,7 @@ def test_tar_traversal_rejected(self, catalog, tmp_path): def test_tar_benign_succeeds(self, catalog, tmp_path): """A well-formed tar.gz extracts correctly.""" import io + archive = tmp_path / "good.tar.gz" with tarfile.open(archive, "w:gz") as tf: data = b"hello" @@ -3206,6 +3239,7 @@ def test_zip_benign_succeeds(self, catalog, tmp_path): assert (extract_dir / "pkg" / "hello.txt").read_text() == "world" + class TestPluginCatalogUpdatePluginVersionRegistry: """Tests for PluginCatalog.update_plugin_version_registry method.""" @@ -3213,20 +3247,20 @@ def test_update_plugin_version_registry_creates_new_file(self, tmp_path, mock_gi """Test creating a new versions.json file when none exists.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + manifest = create_test_manifest(name="test_plugin", version="1.0.0") relpath = Path("plugins/test_plugin") - + catalog.update_plugin_version_registry(manifest, relpath) - + # Verify file was created versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" assert versions_file.exists() - + # Verify content with versions_file.open("r") as f: data = json.load(f) - + assert len(data["versions"]) == 1 assert data["versions"][0]["version"] == "1.0.0" assert data["versions"][0]["manifest_file"] == str(relpath) @@ -3236,22 +3270,22 @@ def test_update_plugin_version_registry_adds_new_version(self, tmp_path, mock_gi """Test adding a new version to existing registry.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create initial version manifest1 = create_test_manifest(name="test_plugin", version="1.0.0") relpath1 = Path("plugins/test_plugin") catalog.update_plugin_version_registry(manifest1, relpath1) - + # Add new version manifest2 = create_test_manifest(name="test_plugin", version="2.0.0") relpath2 = Path("plugins/test_plugin") catalog.update_plugin_version_registry(manifest2, relpath2) - + # Verify both versions exist versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" with versions_file.open("r") as f: data = json.load(f) - + assert len(data["versions"]) == 2 versions = [v["version"] for v in data["versions"]] assert "1.0.0" in versions @@ -3262,19 +3296,19 @@ def test_update_plugin_version_registry_handles_duplicate_version(self, tmp_path """Test that duplicate versions are not added.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + manifest = create_test_manifest(name="test_plugin", version="1.0.0") relpath = Path("plugins/test_plugin") - + # Add same version twice catalog.update_plugin_version_registry(manifest, relpath) catalog.update_plugin_version_registry(manifest, relpath) - + # Verify only one version exists versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" with versions_file.open("r") as f: data = json.load(f) - + assert len(data["versions"]) == 1 assert data["versions"][0]["version"] == "1.0.0" @@ -3282,24 +3316,24 @@ def test_update_plugin_version_registry_updates_latest_correctly(self, tmp_path, """Test that latest version is updated correctly when adding versions out of order.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Add version 2.0.0 first manifest2 = create_test_manifest(name="test_plugin", version="2.0.0") catalog.update_plugin_version_registry(manifest2, Path("plugins/test_plugin")) - + # Add version 1.0.0 manifest1 = create_test_manifest(name="test_plugin", version="1.0.0") catalog.update_plugin_version_registry(manifest1, Path("plugins/test_plugin")) - + # Add version 3.0.0 manifest3 = create_test_manifest(name="test_plugin", version="3.0.0") catalog.update_plugin_version_registry(manifest3, Path("plugins/test_plugin")) - + # Verify latest is 3.0.0 versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" with versions_file.open("r") as f: data = json.load(f) - + assert data["latest"]["version"] == "3.0.0" assert len(data["versions"]) == 3 @@ -3307,20 +3341,20 @@ def test_update_plugin_version_registry_with_prerelease_versions(self, tmp_path, """Test handling of pre-release versions.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Add stable version manifest1 = create_test_manifest(name="test_plugin", version="1.0.0") catalog.update_plugin_version_registry(manifest1, Path("plugins/test_plugin")) - + # Add pre-release version manifest2 = create_test_manifest(name="test_plugin", version="2.0.0rc1") catalog.update_plugin_version_registry(manifest2, Path("plugins/test_plugin")) - + # Verify latest is the rc version (higher version number) versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" with versions_file.open("r") as f: data = json.load(f) - + assert len(data["versions"]) == 2 assert data["latest"]["version"] == "2.0.0rc1" @@ -3328,20 +3362,20 @@ def test_update_plugin_version_registry_with_dev_versions(self, tmp_path, mock_g """Test handling of development versions.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Add dev version manifest1 = create_test_manifest(name="test_plugin", version="1.0.0.dev1") catalog.update_plugin_version_registry(manifest1, Path("plugins/test_plugin")) - + # Add stable version manifest2 = create_test_manifest(name="test_plugin", version="1.0.0") catalog.update_plugin_version_registry(manifest2, Path("plugins/test_plugin")) - + # Verify latest is stable version versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" with versions_file.open("r") as f: data = json.load(f) - + assert len(data["versions"]) == 2 assert data["latest"]["version"] == "1.0.0" @@ -3349,12 +3383,12 @@ def test_update_plugin_version_registry_preserves_existing_data(self, tmp_path, """Test that existing version data is preserved when adding new versions.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Manually create a versions.json with additional metadata versions_dir = tmp_path / "catalog" / "test_plugin" versions_dir.mkdir(parents=True) versions_file = versions_dir / "versions.json" - + initial_data = { "latest": { "version": "1.0.0", @@ -3362,7 +3396,7 @@ def test_update_plugin_version_registry_preserves_existing_data(self, tmp_path, "manifest_file": "plugins/test_plugin", "deprecated": False, "breaking_changes": False, - "changelog": "Initial release" + "changelog": "Initial release", }, "versions": [ { @@ -3371,20 +3405,20 @@ def test_update_plugin_version_registry_preserves_existing_data(self, tmp_path, "manifest_file": "plugins/test_plugin", "deprecated": False, "breaking_changes": False, - "changelog": "Initial release" + "changelog": "Initial release", } - ] + ], } versions_file.write_text(json.dumps(initial_data, indent=2)) - + # Add new version manifest2 = create_test_manifest(name="test_plugin", version="2.0.0") catalog.update_plugin_version_registry(manifest2, Path("plugins/test_plugin")) - + # Verify old version data is preserved with versions_file.open("r") as f: data = json.load(f) - + assert len(data["versions"]) == 2 old_version = next(v for v in data["versions"] if v["version"] == "1.0.0") assert old_version["changelog"] == "Initial release" @@ -3394,18 +3428,18 @@ def test_update_plugin_version_registry_with_complex_version_ordering(self, tmp_ """Test version ordering with complex version strings.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Add versions in random order versions = ["1.0.0", "2.0.0rc1", "1.5.0", "2.0.0", "1.0.1", "2.1.0a1"] for version in versions: manifest = create_test_manifest(name="test_plugin", version=version) catalog.update_plugin_version_registry(manifest, Path("plugins/test_plugin")) - + # Verify latest is 2.1.0a1 (highest version) versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" with versions_file.open("r") as f: data = json.load(f) - + assert len(data["versions"]) == 6 assert data["latest"]["version"] == "2.1.0a1" @@ -3413,11 +3447,11 @@ def test_update_plugin_version_registry_creates_parent_directories(self, tmp_pat """Test that parent directories are created if they don't exist.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Don't create the directory beforehand manifest = create_test_manifest(name="test_plugin", version="1.0.0") catalog.update_plugin_version_registry(manifest, Path("plugins/test_plugin")) - + # Verify directory and file were created versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" assert versions_file.exists() @@ -3427,13 +3461,13 @@ def test_update_plugin_version_registry_with_invalid_existing_json(self, tmp_pat """Test handling of corrupted existing versions.json file.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Create corrupted versions.json versions_dir = tmp_path / "catalog" / "test_plugin" versions_dir.mkdir(parents=True) versions_file = versions_dir / "versions.json" versions_file.write_text("invalid json content") - + # Attempt to update should raise an error manifest = create_test_manifest(name="test_plugin", version="1.0.0") with pytest.raises(json.JSONDecodeError): @@ -3443,56 +3477,56 @@ def test_update_plugin_version_registry_timestamp_format(self, tmp_path, mock_gi """Test that timestamp is in correct ISO format with Z suffix.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + manifest = create_test_manifest(name="test_plugin", version="1.0.0") catalog.update_plugin_version_registry(manifest, Path("plugins/test_plugin")) - + versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" with versions_file.open("r") as f: data = json.load(f) - + released = data["versions"][0]["released"] # Verify format: ends with Z and contains T assert released.endswith("Z") assert "T" in released # Verify it's a valid ISO format from datetime import datetime + datetime.fromisoformat(released.replace("Z", "+00:00")) def test_update_plugin_version_registry_with_epoch_versions(self, tmp_path, mock_github_env): """Test handling of versions with epochs.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + # Add version with epoch manifest1 = create_test_manifest(name="test_plugin", version="1!1.0.0") catalog.update_plugin_version_registry(manifest1, Path("plugins/test_plugin")) - + # Add version without epoch (should be lower) manifest2 = create_test_manifest(name="test_plugin", version="2.0.0") catalog.update_plugin_version_registry(manifest2, Path("plugins/test_plugin")) - + # Verify epoch version is latest versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" with versions_file.open("r") as f: data = json.load(f) - + assert data["latest"]["version"] == "1!1.0.0" def test_update_plugin_version_registry_relpath_stored_correctly(self, tmp_path, mock_github_env): """Test that relative path is stored correctly in manifest_file.""" catalog = PluginCatalog() catalog.catalog_folder = str(tmp_path / "catalog") - + manifest = create_test_manifest(name="test_plugin", version="1.0.0") relpath = Path("custom/path/to/plugin") - + catalog.update_plugin_version_registry(manifest, relpath) - + versions_file = tmp_path / "catalog" / "test_plugin" / "versions.json" with versions_file.open("r") as f: data = json.load(f) - class TestPluginCatalogVerMethod: @@ -3568,11 +3602,12 @@ def test_ver_version_with_local_identifier(self, mock_github_env): def test_ver_logs_debug_on_invalid_version(self, mock_github_env, caplog): """Test that debug log is created for invalid versions.""" import logging + catalog = PluginCatalog() - + with caplog.at_level(logging.DEBUG): catalog._ver("not-a-version") - + assert "Could not parse version" in caplog.text assert "treating as lowest" in caplog.text @@ -3601,7 +3636,7 @@ def test_ver_comparison_works_correctly(self, mock_github_env): v1 = catalog._ver("1.0.0") v2 = catalog._ver("2.0.0") v_invalid = catalog._ver("invalid") - + assert v1 < v2 assert v_invalid < v1 assert v_invalid == catalog._ver("0") @@ -3612,7 +3647,7 @@ def test_ver_prerelease_comparison(self, mock_github_env): v_stable = catalog._ver("1.0.0") v_rc = catalog._ver("1.0.0rc1") v_dev = catalog._ver("1.0.0.dev1") - + assert v_dev < v_rc < v_stable def test_ver_epoch_comparison(self, mock_github_env): @@ -3620,6 +3655,392 @@ def test_ver_epoch_comparison(self, mock_github_env): catalog = PluginCatalog() v_no_epoch = catalog._ver("2.0.0") v_with_epoch = catalog._ver("1!1.0.0") - + # Epoch takes precedence assert v_with_epoch > v_no_epoch + + +class TestClassifyPluginKind: + """Tests for classify_plugin_kind (U1: FQN kind detection).""" + + @pytest.mark.parametrize( + "kind", + ["builtin", "native", "wasm", "external", "isolated_venv", "PDP"], + ) + def test_known_kinds_classify_as_known(self, kind): + """Each recognized kind classifies as KIND_KNOWN.""" + from cpex.tools.catalog import KIND_KNOWN, classify_plugin_kind + + assert classify_plugin_kind(kind) == KIND_KNOWN + + def test_fqn_class_path_classifies_as_fqn(self): + """A dotted class path that is not a known kind is an FQN to convert.""" + from cpex.tools.catalog import KIND_FQN, classify_plugin_kind + + assert classify_plugin_kind("cpex_pii_filter.pii_filter.PIIFilterPlugin") == KIND_FQN + + def test_single_segment_rejected(self): + """A bare token with no dots is not a class path.""" + from cpex.tools.catalog import KIND_REJECT, classify_plugin_kind + + assert classify_plugin_kind("PIIFilterPlugin") == KIND_REJECT + + def test_typo_kind_rejected(self): + """A misspelled known kind is rejected, not silently converted.""" + from cpex.tools.catalog import KIND_REJECT, classify_plugin_kind + + assert classify_plugin_kind("isolate_venv") == KIND_REJECT + + def test_lowercase_final_segment_rejected(self): + """A dotted path whose final segment is not class-shaped is rejected.""" + from cpex.tools.catalog import KIND_REJECT, classify_plugin_kind + + assert classify_plugin_kind("a.b.c") == KIND_REJECT + + @pytest.mark.parametrize("kind", [None, "", " "]) + def test_empty_or_whitespace_rejected(self, kind): + """Missing or blank kind is rejected.""" + from cpex.tools.catalog import KIND_REJECT, classify_plugin_kind + + assert classify_plugin_kind(kind) == KIND_REJECT + + def test_invalid_identifier_segment_rejected(self): + """A dotted path with a non-identifier segment is rejected.""" + from cpex.tools.catalog import KIND_REJECT, classify_plugin_kind + + assert classify_plugin_kind("pkg.1module.ClassName") == KIND_REJECT + + def test_supported_kinds_message_names_all_kinds(self): + """The rejection message names every supported kind.""" + from cpex.tools.catalog import supported_kinds_message + + msg = supported_kinds_message() + for kind in ["builtin", "native", "wasm", "external", "isolated_venv", "PDP"]: + assert kind in msg + + +class TestConvertFqnKind: + """Tests for convert_fqn_kind_in_place and normalize integration (U2).""" + + def test_fqn_manifest_converted(self): + """A bare-FQN kind moves into class_name and kind becomes isolated_venv.""" + from cpex.tools.catalog import convert_fqn_kind_in_place + + data = { + "name": "cpex-pii-filter", + "kind": "cpex_pii_filter.pii_filter.PIIFilterPlugin", + "default_config": {"detect_ssn": True}, + } + convert_fqn_kind_in_place(data) + + assert data["kind"] == "isolated_venv" + assert data["default_config"]["class_name"] == "cpex_pii_filter.pii_filter.PIIFilterPlugin" + # Other config fields preserved. + assert data["default_config"]["detect_ssn"] is True + + def test_fqn_manifest_without_default_config(self): + """Conversion creates default_config when absent.""" + from cpex.tools.catalog import convert_fqn_kind_in_place + + data = {"name": "p", "kind": "pkg.mod.MyPlugin"} + convert_fqn_kind_in_place(data) + + assert data["kind"] == "isolated_venv" + assert data["default_config"]["class_name"] == "pkg.mod.MyPlugin" + + def test_already_isolated_venv_untouched(self): + """An isolated_venv manifest with class_name is idempotent.""" + from cpex.tools.catalog import convert_fqn_kind_in_place + + data = { + "name": "p", + "kind": "isolated_venv", + "default_config": {"class_name": "pkg.mod.Plugin", "requirements_file": "requirements.txt"}, + } + convert_fqn_kind_in_place(dict(data)) # copy to be safe + result = convert_fqn_kind_in_place(data) + + assert result["kind"] == "isolated_venv" + assert result["default_config"]["class_name"] == "pkg.mod.Plugin" + + def test_explicit_class_name_wins_over_fqn(self): + """When both an FQN kind and class_name exist, keep explicit class_name.""" + from cpex.tools.catalog import convert_fqn_kind_in_place + + data = { + "name": "p", + "kind": "pkg.mod.FromKind", + "default_config": {"class_name": "pkg.mod.Explicit"}, + } + convert_fqn_kind_in_place(data) + + assert data["kind"] == "isolated_venv" + assert data["default_config"]["class_name"] == "pkg.mod.Explicit" + + def test_unknown_non_fqn_kind_raises(self): + """A non-FQN unknown kind raises with the supported-kinds message.""" + from cpex.tools.catalog import convert_fqn_kind_in_place + + data = {"name": "p", "kind": "isolate_venv"} + with pytest.raises(ValueError, match="isolated_venv"): + convert_fqn_kind_in_place(data) + + def test_no_convert_leaves_fqn_kind_untouched(self): + """convert=False (--no-convert) keeps a bare-FQN kind as declared.""" + from cpex.tools.catalog import convert_fqn_kind_in_place + + data = {"name": "p", "kind": "pkg.mod.MyPlugin", "default_config": {"a": 1}} + result = convert_fqn_kind_in_place(data, convert=False) + + # Kind is not rewritten and no class_name is injected. + assert result["kind"] == "pkg.mod.MyPlugin" + assert "class_name" not in result["default_config"] + assert result["default_config"]["a"] == 1 + + def test_no_convert_warns_and_passes_through_reject(self, caplog): + """convert=False softens an unknown/reject kind from raise to warn.""" + import logging + + from cpex.tools.catalog import convert_fqn_kind_in_place + + data = {"name": "p", "kind": "isolate_venv"} # typo -> KIND_REJECT + with caplog.at_level(logging.WARNING): + result = convert_fqn_kind_in_place(data, convert=False) + + # No exception; kind left unchanged; a warning was emitted. + assert result["kind"] == "isolate_venv" + assert any("isolated_venv" in rec.message for rec in caplog.records) + + def test_no_convert_known_kind_untouched(self): + """convert=False still passes a known kind through unchanged.""" + from cpex.tools.catalog import convert_fqn_kind_in_place + + data = {"name": "p", "kind": "external"} + result = convert_fqn_kind_in_place(data, convert=False) + assert result["kind"] == "external" + + def test_normalize_no_convert_keeps_fqn_kind(self, mock_github_env): + """_normalize_manifest_data(convert=False) leaves the FQN kind in place.""" + catalog = PluginCatalog() + manifest_data = { + "kind": "cpex_pii_filter.pii_filter.PIIFilterPlugin", + "description": "PII filter", + "author": "ContextForge", + "version": "0.3.6", + "available_hooks": ["tool_pre_invoke"], + "default_config": {"detect_ssn": True}, + } + manifest = catalog._normalize_manifest_data(manifest_data, "cpex-pii-filter", ">=0.3.0", convert=False) + + assert manifest.kind == "cpex_pii_filter.pii_filter.PIIFilterPlugin" + assert "class_name" not in manifest.default_config + + def test_normalize_converts_fqn_and_normalizes_default_configs(self, mock_github_env): + """_normalize_manifest_data converts FQN after normalizing legacy default_configs.""" + catalog = PluginCatalog() + manifest_data = { + "kind": "cpex_pii_filter.pii_filter.PIIFilterPlugin", + "description": "PII filter", + "author": "ContextForge", + "version": "0.3.6", + "available_hooks": ["tool_pre_invoke"], + "default_configs": {"detect_ssn": True}, # legacy plural key + } + manifest = catalog._normalize_manifest_data(manifest_data, "cpex-pii-filter", ">=0.3.0") + + assert manifest.kind == "isolated_venv" + assert manifest.default_config["class_name"] == "cpex_pii_filter.pii_filter.PIIFilterPlugin" + assert manifest.default_config["detect_ssn"] is True + + def test_normalize_rejects_typo_kind(self, mock_github_env): + """_normalize_manifest_data surfaces the rejection for a typo'd kind.""" + catalog = PluginCatalog() + manifest_data = { + "kind": "isolate_venv", + "description": "d", + "author": "a", + "version": "1.0.0", + "available_hooks": [], + "default_config": {}, + } + with pytest.raises(RuntimeError, match="isolated_venv"): + catalog._normalize_manifest_data(manifest_data, "p", None) + + def test_transform_leaves_unrecognized_kind_unconverted(self, mock_github_env): + """Catalog scan must not drop a manifest over an unrecognized kind (Finding #2). + + _transform_manifest_data is used during catalog population; a reject there + should leave the kind unconverted rather than raising and silently dropping + the plugin. The install path still raises (covered above). + """ + catalog = PluginCatalog() + content = {"kind": "some_odd_token", "default_configs": {}} + result = catalog._transform_manifest_data( + content, name="odd", member=None, repo_url=httpx.URL("https://github.com/org/repo") + ) + # Kind is left as-is; no exception, manifest not dropped. + assert result["kind"] == "some_odd_token" + + +class TestInstallPackageIntoVenv: + """Tests for installing the plugin package into the isolated venv (U4).""" + + def _isolated_manifest_file(self, tmp_path, requirements_file=None): + extract_dir = tmp_path / "extracted" + extract_dir.mkdir(exist_ok=True) + package_dir = extract_dir / "test_package" + package_dir.mkdir(exist_ok=True) + default_config = {"class_name": "test_package.mod.Plugin"} + if requirements_file: + default_config["requirements_file"] = requirements_file + manifest_data = { + "name": "test_package", + "version": "1.0.0", + "kind": "isolated_venv", + "description": "Test", + "author": "Test Author", + "tags": ["test"], + "available_hooks": ["tool_pre_invoke"], + "default_config": default_config, + } + manifest_file = package_dir / "plugin-manifest.yaml" + manifest_file.write_text(yaml.safe_dump(manifest_data)) + return extract_dir, manifest_file + + def test_helper_builds_pip_install_command(self, tmp_path, mock_github_env): + """_install_package_into_venv runs pip install in the venv python with the given args.""" + catalog = PluginCatalog() + plugin_path = tmp_path / "plug" + with ( + patch.object(PluginCatalog, "_get_venv_python_executable", return_value="/venv/bin/python"), + patch("cpex.tools.catalog.subprocess.run") as mock_run, + ): + catalog._install_package_into_venv(plugin_path, ["some-pkg>=1.0"]) + + args = mock_run.call_args[0][0] + assert args == ["/venv/bin/python", "-m", "pip", "install", "some-pkg>=1.0"] + + def test_pypi_installs_package_into_venv_with_constraint(self, tmp_path, mock_github_env): + """PyPI isolated install installs the package into the venv with its version constraint.""" + extract_dir, manifest_file = self._isolated_manifest_file(tmp_path) + with ( + patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), + patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", return_value=manifest_file), + patch("cpex.tools.catalog.PluginCatalog._handle_plugin_installation", return_value=tmp_path / "plug"), + patch("cpex.tools.catalog.PluginCatalog._install_package_into_venv") as mock_venv_install, + patch("cpex.tools.catalog.PluginCatalog._finalize_plugin_installation", return_value=tmp_path / "plug"), + patch("shutil.rmtree"), + ): + catalog = PluginCatalog() + catalog.catalog_folder = str(tmp_path / "catalog") + catalog.install_from_pypi("test_package", version_constraint=">=0.2.0") + + mock_venv_install.assert_called_once() + pth, pip_args = mock_venv_install.call_args[0] + assert pip_args == ["test_package>=0.2.0"] + + def test_test_pypi_uses_test_index(self, tmp_path, mock_github_env): + """test-pypi isolated install passes the test index URL.""" + extract_dir, manifest_file = self._isolated_manifest_file(tmp_path) + with ( + patch("cpex.tools.catalog.PluginCatalog._download_package_to_temp", return_value=extract_dir), + patch("cpex.tools.catalog.PluginCatalog._find_manifest_in_extracted_package", return_value=manifest_file), + patch("cpex.tools.catalog.PluginCatalog._handle_plugin_installation", return_value=tmp_path / "plug"), + patch("cpex.tools.catalog.PluginCatalog._install_package_into_venv") as mock_venv_install, + patch("cpex.tools.catalog.PluginCatalog._finalize_plugin_installation", return_value=tmp_path / "plug"), + patch("shutil.rmtree"), + ): + catalog = PluginCatalog() + catalog.catalog_folder = str(tmp_path / "catalog") + catalog.install_from_pypi("test_package", use_pytest=True) + + _pth, pip_args = mock_venv_install.call_args[0] + assert "--index-url" in pip_args + assert "https://test.pypi.org/simple/" in pip_args + assert "test_package" in pip_args + + def test_persist_manifest_to_plugin_dir(self, tmp_path, mock_github_env): + """The converted manifest is written under plugins// (U5, R3). + + The filename is keyed on the full class name (#4), not the shared + class_root, so multi-plugin packages do not collide on one manifest. + """ + from cpex.framework.utils import manifest_filename_for_class + + catalog = PluginCatalog() + catalog.plugin_folder = str(tmp_path / "plugins") + manifest = create_test_manifest( + name="cpex-pii-filter", + kind="isolated_venv", + default_config={"class_name": "cpex_pii_filter.pii_filter.PIIFilterPlugin"}, + ) + + result = catalog._persist_manifest_to_plugin_dir(manifest) + + expected = ( + Path(catalog.plugin_folder) + / "cpex_pii_filter" + / manifest_filename_for_class("cpex_pii_filter.pii_filter.PIIFilterPlugin") + ) + assert result == expected + assert expected.exists() + written = yaml.safe_load(expected.read_text()) + assert written["kind"] == "isolated_venv" + assert written["default_config"]["class_name"] == "cpex_pii_filter.pii_filter.PIIFilterPlugin" + + def test_persist_manifest_multi_plugin_package_no_collision(self, tmp_path, mock_github_env): + """Two plugins in one package get distinct manifest files in the shared dir (#4).""" + catalog = PluginCatalog() + catalog.plugin_folder = str(tmp_path / "plugins") + + manifest_a = create_test_manifest( + name="pkg-a", + kind="isolated_venv", + default_config={"class_name": "cpex_pkg.a.PluginA"}, + ) + manifest_b = create_test_manifest( + name="pkg-b", + kind="isolated_venv", + default_config={"class_name": "cpex_pkg.b.PluginB"}, + ) + + path_a = catalog._persist_manifest_to_plugin_dir(manifest_a) + path_b = catalog._persist_manifest_to_plugin_dir(manifest_b) + + # Same shared class_root directory, distinct manifest files. + assert path_a.parent == path_b.parent + assert path_a != path_b + assert path_a.exists() and path_b.exists() + + def test_persist_manifest_to_plugin_dir_skips_non_isolated(self, tmp_path, mock_github_env): + """Non-isolated plugins are not persisted to a plugin dir.""" + catalog = PluginCatalog() + catalog.plugin_folder = str(tmp_path / "plugins") + manifest = create_test_manifest(name="p", kind="native") + + assert catalog._persist_manifest_to_plugin_dir(manifest) is None + + def test_monorepo_installs_package_into_venv_from_source(self, tmp_path, mock_github_env): + """Monorepo isolated install installs the subdirectory source into the venv.""" + from cpex.framework.models import Monorepo + + catalog = PluginCatalog() + manifest = create_test_manifest( + name="test_package", + kind="isolated_venv", + default_config={"class_name": "test_package.mod.Plugin"}, + ) + manifest.monorepo = Monorepo( + package_source="https://github.com/org/repo#subdirectory=plugins/test_package", + repo_url="https://github.com/org/repo", + package_folder="plugins/test_package", + ) + with ( + patch("cpex.tools.catalog.PluginCatalog._download_monorepo_folder_to_temp", return_value=tmp_path / "pkg"), + patch("cpex.tools.catalog.PluginCatalog._initialize_isolated_venv", return_value=tmp_path / "plug"), + patch("cpex.tools.catalog.PluginCatalog._install_package_into_venv") as mock_venv_install, + ): + catalog.install_folder_via_pip(manifest) + + _pth, pip_args = mock_venv_install.call_args[0] + assert pip_args == ["git+https://github.com/org/repo#subdirectory=plugins/test_package"] diff --git a/tests/unit/cpex/tools/test_cli.py b/tests/unit/cpex/tools/test_cli.py index 524693c9..e155051f 100644 --- a/tests/unit/cpex/tools/test_cli.py +++ b/tests/unit/cpex/tools/test_cli.py @@ -9,16 +9,17 @@ # Standard import json -import tempfile from pathlib import Path -from unittest.mock import MagicMock, Mock, patch, mock_open +from unittest.mock import MagicMock, Mock, patch -import pytest # We use typer's CliRunner for testing typer apps import click +import pytest import typer from typer.testing import CliRunner +from cpex.framework.models import Config, Monorepo, PluginConfig, PluginManifest, PluginMode, PyPiRepo + # Third-Party # First-Party from cpex.tools.cli import ( @@ -26,22 +27,23 @@ DEFAULT_AUTHOR_NAME, DEFAULT_TEMPLATE_URL, LOCAL_TEMPLATES_DIR, + _plugin_name_from_source, + _should_skip_reinstall, app, command_exists, git_user_email, git_user_name, - list_registered_plugins, - install_from_manifest, - install, - search, info, + install, + install_from_manifest, instance_name_is_unique, - update_plugins_config_yaml, + list_registered_plugins, remove_from_plugins_config_yaml, + search, uninstall, + update_plugins_config_yaml, ) from cpex.tools.plugin_registry import PluginRegistry -from cpex.framework.models import PluginManifest, Monorepo, Config, PluginConfig, PluginMode, PyPiRepo runner = CliRunner() @@ -53,6 +55,7 @@ # Fixtures # --------------------------------------------------------------------------- + @pytest.fixture def temp_registry_dir(tmp_path, monkeypatch): """Fixture to ensure all tests use a temporary directory for the plugin registry.""" @@ -74,7 +77,11 @@ def create_test_manifest(**kwargs): "tags": ["test"], "available_hooks": ["tools"], "default_config": {}, - "monorepo": Monorepo(package_source="https://example.com/repo#subdirectory=plugin", repo_url="https://example.com/repo", package_folder="plugin"), + "monorepo": Monorepo( + package_source="https://example.com/repo#subdirectory=plugin", + repo_url="https://example.com/repo", + package_folder="plugin", + ), } defaults.update(kwargs) return PluginManifest(**defaults) @@ -463,7 +470,6 @@ def test_main_invokes_app(self): mock_app.assert_called_once() - # --------------------------------------------------------------------------- # Plugin management function tests # --------------------------------------------------------------------------- @@ -516,13 +522,13 @@ class TestUpdatePluginRegistry: def test_creates_new_registry_if_not_exists(self, temp_registry_dir): """Test creating a new registry when file doesn't exist.""" manifest = create_test_manifest() - + mock_catalog = Mock() with ( - patch("cpex.tools.cli.git_user_name", return_value="test_user"), - patch("cpex.tools.plugin_registry.find_package_path", return_value=Path("/fake/path/to/plugin")), - ): + patch("cpex.tools.cli.git_user_name", return_value="test_user"), + patch("cpex.tools.plugin_registry.find_package_path", return_value=Path("/fake/path/to/plugin")), + ): plugin_registry = PluginRegistry() plugin_registry.update(manifest, "monorepo", mock_catalog, "test_user") registry_file = temp_registry_dir / "installed-plugins.json" @@ -538,9 +544,13 @@ def test_updates_existing_registry(self, temp_registry_dir): name="new_plugin", version="2.0.0", kind="external", - monorepo=Monorepo(package_source="https://example.com/repo#subdirectory=new_plugin", repo_url="https://example.com/repo", package_folder="new_plugin"), + monorepo=Monorepo( + package_source="https://example.com/repo#subdirectory=new_plugin", + repo_url="https://example.com/repo", + package_folder="new_plugin", + ), ) - + mock_catalog = Mock() with ( @@ -604,7 +614,7 @@ def test_update_raises_for_invalid_installation_type(self, temp_registry_dir): with pytest.raises(ValueError, match="Invalid installation type: invalid"): plugin_registry.update(manifest, "invalid", Mock(), "test_user") - + def test_update_with_local_installation_and_explicit_plugin_path(self, temp_registry_dir): """Test registry update for local installation with explicit plugin_path.""" manifest = create_test_manifest(monorepo=None, package_info=None) @@ -638,13 +648,17 @@ def test_update_uses_find_package_path_when_plugin_path_not_provided(self, temp_ """Test registry update falls back to find_package_path when plugin_path is omitted.""" manifest = create_test_manifest() - with patch("cpex.tools.plugin_registry.find_package_path", return_value=Path("/fake/path/from/find_package_path")): + with patch( + "cpex.tools.plugin_registry.find_package_path", return_value=Path("/fake/path/from/find_package_path") + ): plugin_registry = PluginRegistry() plugin_registry.update(manifest, "monorepo", Mock(), "test_user") registry_file = temp_registry_dir / "installed-plugins.json" updated_data = json.loads(registry_file.read_text()) - assert updated_data["plugins"][0]["installation_path"] == str(Path("/fake/path/from/find_package_path").resolve()) + assert updated_data["plugins"][0]["installation_path"] == str( + Path("/fake/path/from/find_package_path").resolve() + ) def test_has_returns_true_when_plugin_present(self, temp_registry_dir): """Test has() returns True for an installed plugin.""" @@ -686,24 +700,18 @@ class TestInstanceNameIsUnique: def test_returns_true_for_unique_name(self): """Test that unique names return True.""" existing_plugin = PluginConfig( - name="existing_plugin", - kind="test.plugin", - mode=PluginMode.SEQUENTIAL, - priority=100 + name="existing_plugin", kind="test.plugin", mode=PluginMode.SEQUENTIAL, priority=100 ) - + config = Config(plugins=[existing_plugin]) assert instance_name_is_unique(config, "new_plugin") is True def test_returns_false_for_duplicate_name(self): """Test that duplicate names return False.""" existing_plugin = PluginConfig( - name="existing_plugin", - kind="test.plugin", - mode=PluginMode.SEQUENTIAL, - priority=100 + name="existing_plugin", kind="test.plugin", mode=PluginMode.SEQUENTIAL, priority=100 ) - + config = Config(plugins=[existing_plugin]) assert instance_name_is_unique(config, "existing_plugin") is False @@ -720,19 +728,20 @@ def test_updates_config_with_unique_name(self, tmp_path): """Test updating config with a unique plugin name.""" manifest = create_test_manifest() config_file = tmp_path / "config.yaml" - + mock_config = Config(plugins=[]) - + with ( patch("cpex.tools.cli.ConfigLoader.load_config", return_value=mock_config), patch("cpex.tools.cli.ConfigSaver.save_config") as mock_save, patch.object(type(manifest), "suggest_instance_name", return_value="test_plugin"), - patch.object(type(manifest), "create_instance_config", return_value=PluginConfig( - name="test_plugin", - kind="test.plugin", - mode=PluginMode.SEQUENTIAL, - priority=100 - )), + patch.object( + type(manifest), + "create_instance_config", + return_value=PluginConfig( + name="test_plugin", kind="test.plugin", mode=PluginMode.SEQUENTIAL, priority=100 + ), + ), ): update_plugins_config_yaml(manifest) mock_save.assert_called_once() @@ -744,26 +753,22 @@ def test_generates_unique_name_when_duplicate(self, tmp_path): """Test that duplicate names get suffixed with counter.""" manifest = create_test_manifest(name="test_plugin") config_file = tmp_path / "config.yaml" - + # Create existing plugin with same suggested name - existing_plugin = PluginConfig( - name="test_plugin", - kind="test.plugin", - mode=PluginMode.SEQUENTIAL, - priority=100 - ) + existing_plugin = PluginConfig(name="test_plugin", kind="test.plugin", mode=PluginMode.SEQUENTIAL, priority=100) mock_config = Config(plugins=[existing_plugin]) - + with ( patch("cpex.tools.cli.ConfigLoader.load_config", return_value=mock_config), patch("cpex.tools.cli.ConfigSaver.save_config") as mock_save, patch.object(type(manifest), "suggest_instance_name", return_value="test_plugin"), - patch.object(type(manifest), "create_instance_config", return_value=PluginConfig( - name="test_plugin_1", - kind="test.plugin", - mode=PluginMode.SEQUENTIAL, - priority=100 - )), + patch.object( + type(manifest), + "create_instance_config", + return_value=PluginConfig( + name="test_plugin_1", kind="test.plugin", mode=PluginMode.SEQUENTIAL, priority=100 + ), + ), ): update_plugins_config_yaml(manifest) mock_save.assert_called_once() @@ -801,7 +806,7 @@ def test_install_git_implementation(self): mock_catalog = Mock() test_manifest = create_test_manifest(name="test_plugin", kind="native") mock_catalog.install_from_git = Mock(return_value=(test_manifest, Path("/path/to/plugin"))) - + with ( patch("cpex.tools.cli._finalize_installation") as mock_finalize, patch("cpex.tools.cli.console") as mock_console, @@ -811,7 +816,7 @@ def test_install_git_implementation(self): mock_status.__enter__ = Mock(return_value=mock_status) mock_status.__exit__ = Mock(return_value=False) mock_console.status = Mock(return_value=mock_status) - + install("test_plugin @ git+https://github.com/example/test_plugin.git", "git", mock_catalog) # Verify install_from_git was called @@ -1035,31 +1040,22 @@ def test_callback_exists(self): callback() - class TestRemoveFromPluginsConfigYaml: """Tests for remove_from_plugins_config_yaml() function.""" def test_removes_plugin_from_config(self, tmp_path): """Test removing a plugin from config.""" config_file = tmp_path / "config.yaml" - + plugin1 = PluginConfig( - name="plugin_to_remove", - kind="test.plugin.remove", - mode=PluginMode.SEQUENTIAL, - priority=100 - ) - plugin2 = PluginConfig( - name="plugin_to_keep", - kind="test.plugin.keep", - mode=PluginMode.SEQUENTIAL, - priority=100 + name="plugin_to_remove", kind="test.plugin.remove", mode=PluginMode.SEQUENTIAL, priority=100 ) + plugin2 = PluginConfig(name="plugin_to_keep", kind="test.plugin.keep", mode=PluginMode.SEQUENTIAL, priority=100) mock_config = Config(plugins=[plugin1, plugin2]) - + # Create a manifest with matching kind manifest = create_test_manifest(name="plugin_to_remove", kind="test.plugin.remove") - + with ( patch("cpex.tools.cli.ConfigLoader.load_config", return_value=mock_config), patch("cpex.tools.cli.ConfigSaver.save_config") as mock_save, @@ -1073,40 +1069,22 @@ def test_removes_plugin_from_config(self, tmp_path): def test_removes_isolated_venv_plugin_from_config(self, tmp_path): """Test removing a plugin from config.""" config_file = tmp_path / "config.yaml" - - config_1 = { - "class_name": "test.plugin.remove", - "requirements_file": "requirements.txt" - } - config_2 = { - "class_name": "test.plugin.keep", - "requirements_file": "requirements.txt" - } + config_1 = {"class_name": "test.plugin.remove", "requirements_file": "requirements.txt"} + + config_2 = {"class_name": "test.plugin.keep", "requirements_file": "requirements.txt"} plugin1 = PluginConfig( - name="plugin_to_remove", - kind="isolated_venv", - mode=PluginMode.SEQUENTIAL, - priority=100, - config=config_1 + name="plugin_to_remove", kind="isolated_venv", mode=PluginMode.SEQUENTIAL, priority=100, config=config_1 ) plugin2 = PluginConfig( - name="plugin_to_keep", - kind="test.plugin.keep", - mode=PluginMode.SEQUENTIAL, - priority=100, - config=config_2 + name="plugin_to_keep", kind="test.plugin.keep", mode=PluginMode.SEQUENTIAL, priority=100, config=config_2 ) mock_config = Config(plugins=[plugin1, plugin2]) - + # Create a manifest with matching kind - manifest = create_test_manifest( - name="plugin_to_remove", - kind="isolated_venv", - default_config=config_1 - ) - + manifest = create_test_manifest(name="plugin_to_remove", kind="isolated_venv", default_config=config_1) + with ( patch("cpex.tools.cli.ConfigLoader.load_config", return_value=mock_config), patch("cpex.tools.cli.ConfigSaver.save_config") as mock_save, @@ -1117,20 +1095,16 @@ def test_removes_isolated_venv_plugin_from_config(self, tmp_path): assert len(mock_config.plugins) == 1 assert mock_config.plugins[0].kind == "test.plugin.keep" - def test_returns_false_when_plugin_not_found(self, tmp_path): """Test that function returns False when plugin not found.""" plugin1 = PluginConfig( - name="existing_plugin", - kind="test.plugin.existing", - mode=PluginMode.SEQUENTIAL, - priority=100 + name="existing_plugin", kind="test.plugin.existing", mode=PluginMode.SEQUENTIAL, priority=100 ) mock_config = Config(plugins=[plugin1]) - + # Create a manifest with non-matching kind manifest = create_test_manifest(name="nonexistent_plugin", kind="test.plugin.nonexistent") - + with ( patch("cpex.tools.cli.ConfigLoader.load_config", return_value=mock_config), patch("cpex.tools.cli.ConfigSaver.save_config") as mock_save, @@ -1143,7 +1117,7 @@ def test_returns_false_when_no_plugins_in_config(self, tmp_path): """Test that function returns False when config has no plugins.""" mock_config = Config(plugins=None) manifest = create_test_manifest(name="any_plugin") - + with patch("cpex.tools.cli.ConfigLoader.load_config", return_value=mock_config): result = remove_from_plugins_config_yaml(manifest) assert result is False @@ -1151,7 +1125,7 @@ def test_returns_false_when_no_plugins_in_config(self, tmp_path): def test_handles_exception_gracefully(self, tmp_path): """Test that function handles exceptions gracefully.""" manifest = create_test_manifest(name="any_plugin") - + with ( patch("cpex.tools.cli.ConfigLoader.load_config", side_effect=Exception("Config error")), patch("cpex.tools.cli.logger") as mock_logger, @@ -1167,7 +1141,7 @@ class TestUninstallFunction: def test_uninstall_plugin_not_found(self, temp_registry_dir): """Test uninstalling a plugin that is not installed.""" mock_catalog = Mock() - + with ( patch("cpex.tools.cli.console") as mock_console, patch("cpex.tools.cli.PluginRegistry") as mock_registry_class, @@ -1176,7 +1150,7 @@ def test_uninstall_plugin_not_found(self, temp_registry_dir): mock_registry = Mock() mock_registry.registry.plugins = [] mock_registry_class.return_value = mock_registry - + with pytest.raises(typer.Exit) as exc_info: uninstall("nonexistent_plugin", mock_catalog) assert exc_info.value.exit_code == 3 # EXIT_NOT_FOUND @@ -1201,9 +1175,9 @@ def test_uninstall_cancelled_by_user(self, temp_registry_dir): ] } registry_file.write_text(json.dumps(registry_data)) - + mock_catalog = Mock() - + with ( patch("cpex.tools.cli.inquirer.prompt", return_value={"confirm": False}), patch("cpex.tools.cli.console") as mock_console, @@ -1230,12 +1204,12 @@ def test_uninstall_success(self, temp_registry_dir): ] } registry_file.write_text(json.dumps(registry_data)) - + mock_catalog = Mock() - + # Create a manifest to return from find test_manifest = create_test_manifest(name="test_plugin", kind="native") - + with ( patch("cpex.tools.cli.inquirer.prompt", return_value={"confirm": True}), patch("cpex.tools.cli.console") as mock_console, @@ -1247,14 +1221,14 @@ def test_uninstall_success(self, temp_registry_dir): mock_catalog_instance.find = Mock(return_value=test_manifest) mock_catalog_instance.uninstall_package = Mock() mock_catalog_class.return_value = mock_catalog_instance - + mock_status = Mock() mock_status.__enter__ = Mock(return_value=mock_status) mock_status.__exit__ = Mock(return_value=False) mock_console.status = Mock(return_value=mock_status) - + uninstall("test_plugin", mock_catalog) - + # Verify uninstall_package was called with both plugin_name and manifest mock_catalog_instance.uninstall_package.assert_called_once_with("test_plugin", test_manifest) mock_remove.assert_called_once_with(test_manifest) @@ -1345,10 +1319,10 @@ def test_plugin_uninstall_command_success(self, temp_registry_dir): ] } registry_file.write_text(json.dumps(registry_data)) - + # Create a manifest to return from find test_manifest = create_test_manifest(name="test_plugin", kind="native") - + with ( patch("cpex.tools.cli.PluginCatalog") as mock_catalog_class, patch("cpex.tools.cli.inquirer.prompt", return_value={"confirm": True}), @@ -1359,12 +1333,12 @@ def test_plugin_uninstall_command_success(self, temp_registry_dir): mock_catalog.uninstall_package = Mock() mock_catalog.find = Mock(return_value=test_manifest) mock_catalog_class.return_value = mock_catalog - + mock_status = Mock() mock_status.__enter__ = Mock(return_value=mock_status) mock_status.__exit__ = Mock(return_value=False) mock_console.status = Mock(return_value=mock_status) - + result = runner.invoke(app, ["plugin", "uninstall", "test_plugin"]) assert result.exit_code == 0 # Verify uninstall_package was called with both plugin_name and manifest @@ -1390,10 +1364,10 @@ class TestCatalogUninstallPackage: def test_uninstall_package_success(self, temp_registry_dir): """Test successful package uninstallation.""" from cpex.tools.catalog import PluginCatalog - + # Create a test manifest test_manifest = create_test_manifest(name="test_package", kind="native") - + with ( patch.dict("os.environ", {"PLUGINS_GITHUB_TOKEN": "test_token"}), patch("cpex.tools.catalog.Github"), @@ -1401,7 +1375,7 @@ def test_uninstall_package_success(self, temp_registry_dir): ): catalog = PluginCatalog() result = catalog.uninstall_package("test_package", test_manifest) - + assert result is True mock_subprocess.assert_called_once() call_args = mock_subprocess.call_args @@ -1412,36 +1386,40 @@ def test_uninstall_package_success(self, temp_registry_dir): def test_uninstall_package_subprocess_error(self, temp_registry_dir): """Test package uninstallation with subprocess error.""" - from cpex.tools.catalog import PluginCatalog import subprocess - + + from cpex.tools.catalog import PluginCatalog + # Create a test manifest test_manifest = create_test_manifest(name="test_package", kind="native") - + with ( patch.dict("os.environ", {"PLUGINS_GITHUB_TOKEN": "test_token"}), patch("cpex.tools.catalog.Github"), - patch("cpex.tools.catalog.subprocess.run", side_effect=subprocess.CalledProcessError(1, ["pip"], stderr="Error")), + patch( + "cpex.tools.catalog.subprocess.run", + side_effect=subprocess.CalledProcessError(1, ["pip"], stderr="Error"), + ), ): catalog = PluginCatalog() - + with pytest.raises(RuntimeError, match="Failed to uninstall"): catalog.uninstall_package("test_package", test_manifest) def test_uninstall_package_unexpected_error(self, temp_registry_dir): """Test package uninstallation with unexpected error.""" from cpex.tools.catalog import PluginCatalog - + # Create a test manifest test_manifest = create_test_manifest(name="test_package", kind="native") - + with ( patch.dict("os.environ", {"PLUGINS_GITHUB_TOKEN": "test_token"}), patch("cpex.tools.catalog.Github"), patch("cpex.tools.catalog.subprocess.run", side_effect=Exception("Unexpected error")), ): catalog = PluginCatalog() - + with pytest.raises(RuntimeError, match="Unexpected error uninstalling"): catalog.uninstall_package("test_package", test_manifest) @@ -1468,10 +1446,10 @@ def test_remove_existing_plugin(self, temp_registry_dir): ] } registry_file.write_text(json.dumps(registry_data)) - + plugin_registry = PluginRegistry() result = plugin_registry.remove("test_plugin") - + assert result is True updated_data = json.loads(registry_file.read_text()) assert len(updated_data["plugins"]) == 0 @@ -1495,10 +1473,10 @@ def test_remove_nonexistent_plugin(self, temp_registry_dir): ] } registry_file.write_text(json.dumps(registry_data)) - + plugin_registry = PluginRegistry() result = plugin_registry.remove("nonexistent_plugin") - + assert result is False updated_data = json.loads(registry_file.read_text()) assert len(updated_data["plugins"]) == 1 @@ -1509,8 +1487,8 @@ class TestInstalledPluginRegistryUnregister: def test_unregister_existing_plugin(self, temp_registry_dir): """Test unregistering an existing plugin.""" - from cpex.framework.models import InstalledPluginRegistry, InstalledPluginInfo, PluginInstallationType - + from cpex.framework.models import InstalledPluginInfo, InstalledPluginRegistry, PluginInstallationType + registry_file = temp_registry_dir / "installed-plugins.json" plugin1 = InstalledPluginInfo( name="plugin1", @@ -1534,18 +1512,18 @@ def test_unregister_existing_plugin(self, temp_registry_dir): package_source="https://example.com/repo/plugin2", editable=False, ) - + registry = InstalledPluginRegistry(plugins=[plugin1, plugin2]) result = registry.unregister_plugin("plugin1") - + assert result is True assert len(registry.plugins) == 1 assert registry.plugins[0].name == "plugin2" def test_unregister_nonexistent_plugin(self, temp_registry_dir): """Test unregistering a plugin that doesn't exist.""" - from cpex.framework.models import InstalledPluginRegistry, InstalledPluginInfo, PluginInstallationType - + from cpex.framework.models import InstalledPluginInfo, InstalledPluginRegistry, PluginInstallationType + plugin1 = InstalledPluginInfo( name="plugin1", kind="native", @@ -1557,10 +1535,10 @@ def test_unregister_nonexistent_plugin(self, temp_registry_dir): package_source="https://example.com/repo/plugin1", editable=False, ) - + registry = InstalledPluginRegistry(plugins=[plugin1]) result = registry.unregister_plugin("nonexistent") - + assert result is False assert len(registry.plugins) == 1 @@ -1568,8 +1546,11 @@ def test_unregister_nonexistent_plugin(self, temp_registry_dir): class TestInstalledPluginRegistryRegisterDedup: """Tests for dedup behaviour of InstalledPluginRegistry.register_plugin().""" - def _make_plugin(self, name="foo", version="1.0.0", path="/path/to/plugin", installed_at="2024-01-01T00:00:00.000000Z"): + def _make_plugin( + self, name="foo", version="1.0.0", path="/path/to/plugin", installed_at="2024-01-01T00:00:00.000000Z" + ): from cpex.framework.models import InstalledPluginInfo, PluginInstallationType + return InstalledPluginInfo( name=name, kind="native", @@ -1585,6 +1566,7 @@ def _make_plugin(self, name="foo", version="1.0.0", path="/path/to/plugin", inst def test_first_register_appends(self, temp_registry_dir): """Registering into an empty registry results in exactly one entry.""" import json + from cpex.framework.models import InstalledPluginRegistry registry = InstalledPluginRegistry() @@ -1599,11 +1581,14 @@ def test_first_register_appends(self, temp_registry_dir): def test_reregister_replaces_existing(self, temp_registry_dir): """Registering a plugin that is already present replaces the old entry.""" import json + from cpex.framework.models import InstalledPluginRegistry registry = InstalledPluginRegistry() registry.register_plugin(self._make_plugin(version="1.0.0", installed_at="2024-01-01T00:00:00.000000Z")) - registry.register_plugin(self._make_plugin(version="2.0.0", path="/new/path", installed_at="2025-06-01T00:00:00.000000Z")) + registry.register_plugin( + self._make_plugin(version="2.0.0", path="/new/path", installed_at="2025-06-01T00:00:00.000000Z") + ) assert len(registry.plugins) == 1 assert registry.plugins[0].version == "2.0.0" @@ -1638,6 +1623,7 @@ class TestInstalledPluginRegistrySaveAtomic: def _make_plugin(self): from cpex.framework.models import InstalledPluginInfo, PluginInstallationType + return InstalledPluginInfo( name="test_plugin", kind="native", @@ -1653,6 +1639,7 @@ def _make_plugin(self): def test_happy_path_no_tmp_litter(self, temp_registry_dir): """save() writes the file and leaves no .tmp siblings.""" from cpex.framework.models import InstalledPluginRegistry + registry = InstalledPluginRegistry() registry.register_plugin(self._make_plugin()) @@ -1666,6 +1653,7 @@ def test_happy_path_no_tmp_litter(self, temp_registry_dir): def test_crash_mid_rename_preserves_original(self, temp_registry_dir, monkeypatch): """If os.replace raises, the original file is untouched and no .tmp remains.""" import os + from cpex.framework.models import InstalledPluginRegistry original_content = b'{"plugins":[]}' @@ -1687,6 +1675,7 @@ def exploding_replace(*args, **kwargs): def test_crash_mid_write_cleans_up_tmp(self, temp_registry_dir, monkeypatch): """If writing to the temp file raises, the temp file is cleaned up.""" import tempfile as _tempfile + from cpex.framework.models import InstalledPluginRegistry original_NamedTemporaryFile = _tempfile.NamedTemporaryFile @@ -1729,16 +1718,16 @@ class TestSelectPluginFromCatalog: def test_returns_none_for_empty_list(self): """Test that function returns None when given empty list.""" from cpex.tools.cli import select_plugin_from_catalog - + result = select_plugin_from_catalog([]) assert result is None def test_returns_none_when_user_cancels(self): """Test that function returns None when user cancels selection.""" from cpex.tools.cli import select_plugin_from_catalog - + manifest = create_test_manifest() - + with patch("cpex.tools.cli.inquirer.prompt", return_value=None): result = select_plugin_from_catalog([manifest]) assert result is None @@ -1750,7 +1739,7 @@ class TestParsePypiSource: def test_parse_package_without_version(self): """Test parsing package name without version constraint.""" from cpex.tools.cli import _parse_pypi_source - + package_name, version_constraint = _parse_pypi_source("my-package") assert package_name == "my-package" assert version_constraint is None @@ -1758,7 +1747,7 @@ def test_parse_package_without_version(self): def test_parse_package_with_version(self): """Test parsing package name with version constraint.""" from cpex.tools.cli import _parse_pypi_source - + package_name, version_constraint = _parse_pypi_source("my-package@>=1.0.0") assert package_name == "my-package" assert version_constraint == ">=1.0.0" @@ -1769,12 +1758,12 @@ class TestFinalizeInstallation: def test_finalize_installation_updates_registry_and_config(self, temp_registry_dir): """Test that finalize_installation updates registry and config.""" - from cpex.tools.cli import _finalize_installation from cpex.tools.catalog import PluginCatalog - + from cpex.tools.cli import _finalize_installation + manifest = create_test_manifest() mock_catalog = Mock(spec=PluginCatalog) - + with ( patch("cpex.tools.cli.PluginRegistry") as mock_registry_class, patch("cpex.tools.cli.update_plugins_config_yaml") as mock_update_config, @@ -1782,9 +1771,9 @@ def test_finalize_installation_updates_registry_and_config(self, temp_registry_d ): mock_registry = Mock() mock_registry_class.return_value = mock_registry - + _finalize_installation(manifest, "pypi", mock_catalog, Path("/test/path")) - + mock_registry.update.assert_called_once() mock_update_config.assert_called_once_with(manifest=manifest) @@ -1794,17 +1783,17 @@ class TestInstallFromLocal: def test_install_from_local_calls_catalog_method(self, temp_registry_dir, tmp_path): """Test that _install_from_local calls catalog.install_from_local.""" - from cpex.tools.cli import _install_from_local from cpex.tools.catalog import PluginCatalog - + from cpex.tools.cli import _install_from_local + source_dir = tmp_path / "my_plugin" source_dir.mkdir() - + manifest = create_test_manifest() manifest.local = str(source_dir) mock_catalog = Mock(spec=PluginCatalog) mock_catalog.install_from_local = Mock(return_value=(manifest, source_dir)) - + with ( patch("cpex.tools.cli.console") as mock_console, patch("cpex.tools.cli.update_plugins_config_yaml") as mock_update_config, @@ -1814,9 +1803,9 @@ def test_install_from_local_calls_catalog_method(self, temp_registry_dir, tmp_pa mock_status.__enter__ = Mock(return_value=mock_status) mock_status.__exit__ = Mock(return_value=False) mock_console.status = Mock(return_value=mock_status) - + _install_from_local(str(source_dir), mock_catalog) - + mock_catalog.install_from_local.assert_called_once() # update_plugins_config_yaml is called inside _finalize_installation (mocked), # not directly from _install_from_local — verify no duplicate direct call. @@ -1829,13 +1818,13 @@ class TestInstallFromMonorepo: def test_returns_early_when_no_plugin_selected(self): """Test that function returns early when user doesn't select a plugin.""" - from cpex.tools.cli import _install_from_monorepo from cpex.tools.catalog import PluginCatalog - + from cpex.tools.cli import _install_from_monorepo + manifest = create_test_manifest() mock_catalog = Mock(spec=PluginCatalog) mock_catalog.search = Mock(return_value=[manifest]) - + with ( patch("cpex.tools.cli.select_plugin_from_catalog", return_value=None), patch("cpex.tools.cli.console"), @@ -1849,30 +1838,87 @@ class TestInstallFromPypi: def test_install_from_pypi_handles_none_manifest(self, temp_registry_dir): """Test that _install_from_pypi handles None manifest gracefully.""" - from cpex.tools.cli import _install_from_pypi from cpex.tools.catalog import PluginCatalog - + from cpex.tools.cli import _install_from_pypi + mock_catalog = Mock(spec=PluginCatalog) mock_catalog.install_from_pypi = Mock(return_value=(None, None)) - + with patch("cpex.tools.cli.console") as mock_console: mock_status = Mock() mock_status.__enter__ = Mock(return_value=mock_status) mock_status.__exit__ = Mock(return_value=False) mock_console.status = Mock(return_value=mock_status) - + _install_from_pypi("test_package", mock_catalog) - + mock_console.print.assert_called_with(":x: Failed to install test_package") +class TestNoConvertThreading: + """--no-convert threads convert=False down to the catalog install methods.""" + + def _mock_catalog(self): + from cpex.tools.catalog import PluginCatalog + + mock_catalog = Mock(spec=PluginCatalog) + mock_catalog.install_from_pypi = Mock(return_value=(None, None)) + return mock_catalog + + def _silence_console(self): + mock_status = Mock() + mock_status.__enter__ = Mock(return_value=mock_status) + mock_status.__exit__ = Mock(return_value=False) + cm = patch("cpex.tools.cli.console") + mock_console = cm.start() + mock_console.status = Mock(return_value=mock_status) + return cm + + def test_pypi_default_converts(self, temp_registry_dir): + """Without --no-convert, the handler passes convert=True.""" + from cpex.tools.cli import _install_from_pypi + + mock_catalog = self._mock_catalog() + cm = self._silence_console() + try: + _install_from_pypi("test_package", mock_catalog) + finally: + cm.stop() + + assert mock_catalog.install_from_pypi.call_args.kwargs["convert"] is True + + def test_pypi_no_convert_threads_false(self, temp_registry_dir): + """convert=False reaches catalog.install_from_pypi.""" + from cpex.tools.cli import _install_from_pypi + + mock_catalog = self._mock_catalog() + cm = self._silence_console() + try: + _install_from_pypi("test_package", mock_catalog, convert=False) + finally: + cm.stop() + + assert mock_catalog.install_from_pypi.call_args.kwargs["convert"] is False + + def test_install_dispatch_forwards_convert(self, temp_registry_dir): + """install(..., convert=False) forwards to the pypi handler.""" + from cpex.tools.catalog import PluginCatalog + from cpex.tools.cli import install + + mock_catalog = Mock(spec=PluginCatalog) + with patch("cpex.tools.cli._install_from_pypi") as mock_handler: + install("test_package", "pypi", catalog=mock_catalog, convert=False) + + assert mock_handler.call_args.kwargs["convert"] is False + + class TestInstallFunctionAdditional: """Additional tests for install() function.""" def test_install_with_unsupported_type_raises_error(self): """Test that install raises typer.Exit for unsupported installation type.""" - from cpex.tools.cli import install from cpex.tools.catalog import PluginCatalog + from cpex.tools.cli import install mock_catalog = Mock(spec=PluginCatalog) @@ -1887,12 +1933,12 @@ class TestVersionsFunction: def test_versions_calls_search(self): """Test that versions() function calls search().""" - from cpex.tools.cli import versions from cpex.tools.catalog import PluginCatalog - + from cpex.tools.cli import versions + mock_catalog = Mock(spec=PluginCatalog) mock_catalog.search = Mock(return_value=[]) - + with patch("cpex.tools.cli.console"): versions("test_plugin", mock_catalog) mock_catalog.search.assert_called_once_with("test_plugin") @@ -1904,28 +1950,28 @@ class TestUpdatePluginsConfigYamlWithNonePlugins: def test_creates_plugins_list_when_none(self, tmp_path): """Test that function creates plugins list when it's None.""" import yaml as yaml_module - + config_file = tmp_path / "config.yaml" config_data = { "plugins": None, # Explicitly None } config_file.write_text(yaml_module.safe_dump(config_data)) - + manifest = create_test_manifest(name="test_plugin") - + with ( patch("cpex.tools.cli.settings") as mock_settings, patch("cpex.tools.cli.ConfigLoader.load_config") as mock_load, patch("cpex.tools.cli.ConfigSaver.save_config") as mock_save, ): mock_settings.config_file = str(config_file) - + # Create a Config object with plugins=None config_obj = Config(plugins=None) mock_load.return_value = config_obj - + update_plugins_config_yaml(manifest) - + # Verify that plugins list was created mock_save.assert_called_once() saved_config = mock_save.call_args[0][0] @@ -1946,12 +1992,12 @@ def test_plugin_search_updates_catalog(self, temp_registry_dir): mock_catalog.update_catalog_with_pyproject = Mock(return_value=False) mock_catalog.search = Mock(return_value=[]) mock_catalog_class.return_value = mock_catalog - + mock_status = Mock() mock_status.__enter__ = Mock(return_value=mock_status) mock_status.__exit__ = Mock(return_value=False) mock_console.status = Mock(return_value=mock_status) - + result = runner.invoke(app, ["plugin", "search", "test"]) assert result.exit_code == 0 mock_catalog.update_catalog_with_pyproject.assert_called_once() @@ -1966,12 +2012,12 @@ def test_plugin_versions_command(self, temp_registry_dir): mock_catalog.update_catalog_with_pyproject = Mock(return_value=False) mock_catalog.search = Mock(return_value=[]) mock_catalog_class.return_value = mock_catalog - + mock_status = Mock() mock_status.__enter__ = Mock(return_value=mock_status) mock_status.__exit__ = Mock(return_value=False) mock_console.status = Mock(return_value=mock_status) - + result = runner.invoke(app, ["plugin", "versions", "test_plugin"]) assert result.exit_code == 0 mock_catalog.search.assert_called_once_with("test_plugin") @@ -2027,5 +2073,109 @@ def test_uninstall_when_manifest_not_found(self, temp_registry_dir): mock_console.print.assert_any_call(":x: Plugin test_plugin not found in catalog.") +def _seed_registry(registry_dir, name, version, kind="isolated_venv", install_type="pypi"): + """Write an installed-plugins.json with one plugin for U6 tests.""" + registry_file = registry_dir / "installed-plugins.json" + registry_file.write_text( + json.dumps( + { + "plugins": [ + { + "name": name, + "kind": kind, + "version": version, + "installation_type": install_type, + "installation_path": "/path/to/plugin", + "installed_at": "2026-01-01T00:00:00.000000Z", + "installed_by": "test_user", + } + ] + } + ) + ) + return registry_file + + +class TestPluginNameFromSource: + """Tests for _plugin_name_from_source (U6).""" + + def test_pypi_source_strips_constraint(self): + assert _plugin_name_from_source("cpex-test-plugin@>=0.2.0", "pypi") == "cpex-test-plugin" + + def test_test_pypi_source_strips_constraint(self): + assert _plugin_name_from_source("cpex-test-plugin@>=0.2.0", "test-pypi") == "cpex-test-plugin" + + def test_git_source_takes_name_before_at(self): + assert ( + _plugin_name_from_source("cpex-test-plugin @ git+https://github.com/o/r@main", "git") == "cpex-test-plugin" + ) + + def test_bare_name_passthrough(self): + assert _plugin_name_from_source("cpex-pii-filter", "monorepo") == "cpex-pii-filter" + + +class TestShouldSkipReinstall: + """Tests for _should_skip_reinstall (U6: repeat-install version compare).""" + + def test_not_installed_proceeds(self, temp_registry_dir): + """A plugin not in the registry always proceeds to install.""" + catalog = Mock() + catalog.find.return_value = None + assert _should_skip_reinstall("new-plugin", "monorepo", catalog) is False + + def test_same_version_skips(self, temp_registry_dir): + """Same installed and catalog version -> skip (no-op).""" + _seed_registry(temp_registry_dir, "cpex-test-plugin", "0.2.0") + catalog = Mock() + catalog.find.return_value = create_test_manifest(name="cpex-test-plugin", version="0.2.0") + assert _should_skip_reinstall("cpex-test-plugin", "monorepo", catalog) is True + + def test_different_version_proceeds(self, temp_registry_dir): + """Catalog version newer than installed -> proceed (upgrade).""" + _seed_registry(temp_registry_dir, "cpex-test-plugin", "0.2.0") + catalog = Mock() + catalog.find.return_value = create_test_manifest(name="cpex-test-plugin", version="0.3.0") + assert _should_skip_reinstall("cpex-test-plugin", "monorepo", catalog) is False + + def test_unresolvable_target_proceeds(self, temp_registry_dir): + """When the catalog has no record (e.g. pypi pre-update), proceed.""" + _seed_registry(temp_registry_dir, "cpex-test-plugin", "0.2.0") + catalog = Mock() + catalog.find.return_value = None + assert _should_skip_reinstall("cpex-test-plugin@>=0.2.0", "pypi", catalog) is False + + def test_version_compare_is_kind_agnostic(self, temp_registry_dir): + """A native (non-converted) plugin at the same version is also skipped.""" + _seed_registry(temp_registry_dir, "native-plugin", "1.0.0", kind="native") + catalog = Mock() + catalog.find.return_value = create_test_manifest(name="native-plugin", version="1.0.0", kind="native") + assert _should_skip_reinstall("native-plugin", "monorepo", catalog) is True + + def test_pypi_explicit_constraint_never_skips(self, temp_registry_dir): + """An explicit version constraint on a pypi source must never skip. + + Regression: the constraint was stripped and unused, and for pypi the + catalog is not refreshed, so catalog.find could return a stale entry + whose version happens to match the installed one — wrongly skipping a + deliberate pin like "foo@==0.3.0". With an explicit constraint we defer + to the real install regardless of the (possibly stale) catalog. + """ + _seed_registry(temp_registry_dir, "cpex-test-plugin", "0.2.0") + catalog = Mock() + # Stale catalog entry that would otherwise trigger a skip. + catalog.find.return_value = create_test_manifest(name="cpex-test-plugin", version="0.2.0") + + assert _should_skip_reinstall("cpex-test-plugin@==0.2.0", "pypi", catalog) is False + assert _should_skip_reinstall("cpex-test-plugin@==0.2.0", "test-pypi", catalog) is False + # The catalog must not even be consulted when a constraint is present. + catalog.find.assert_not_called() + + def test_pypi_without_constraint_still_compares(self, temp_registry_dir): + """A bare pypi name (no constraint) keeps the existing compare behavior.""" + _seed_registry(temp_registry_dir, "cpex-test-plugin", "0.2.0") + catalog = Mock() + catalog.find.return_value = create_test_manifest(name="cpex-test-plugin", version="0.2.0") + assert _should_skip_reinstall("cpex-test-plugin", "pypi", catalog) is True + # Made with Bob