diff --git a/docs/reference/bundles.md b/docs/reference/bundles.md index 613d4e0564..76880b68d9 100644 --- a/docs/reference/bundles.md +++ b/docs/reference/bundles.md @@ -2,7 +2,7 @@ Bundles compose existing Spec Kit components — extensions, presets, workflows, and steps — into a single, versioned, installable unit. Where extensions and presets are primitives, a bundle is a curated stack that declares everything a team or role needs and installs it in one step through each component's own machinery. Bundles add no new runtime behavior of their own: they are a distribution and composition layer over the primitives you already use. -A bundle is described by a `bundle.yml` manifest and is discovered through the same catalog stack as other components. Installing a bundle resolves its declared components against pinned versions, checks for the single cross-bundle conflict point (the active integration), and applies each component idempotently with full provenance tracking so it can be cleanly removed or refreshed later. +A bundle is described by a `bundle.yml` manifest and is discovered through the same catalog stack as other components. Installing a bundle resolves its declared components against pinned versions, checks active-integration and shared-component-version conflicts, and applies each component idempotently with full provenance tracking so it can be cleanly removed or refreshed later. For a concrete starting point, see the [example bundle manifests](https://github.com/github/spec-kit/tree/main/examples/bundles) @@ -53,7 +53,7 @@ specify bundle info | `--offline` | Do not access the network | | `--json` | Emit machine-readable JSON | -Shows full metadata for a bundle along with the **fully expanded component set** it installs — every extension, preset, step, and workflow with its pinned version, plus preset priority and strategy. The output also includes a trust indicator (`verified` vs `community`) so you can judge trust before installing. This preview is the same plan `install` applies, so you can see exactly what will be added before committing. Foreseeable overlaps with components already provided by installed bundles are surfaced here as well. +Shows full metadata for a bundle along with the **fully expanded component set** it installs — every extension, preset, step, and workflow with its pinned version, plus preset priority and strategy. The output also includes a trust indicator (`verified` vs `community`) so you can judge trust before installing. This preview is the same plan `install` applies, so you can see exactly what will be added before committing. Foreseeable overlaps and version conflicts with components already required by installed bundles are surfaced here as well; a bundle can require a component without having installed it. ## Install a Bundle @@ -69,7 +69,9 @@ specify bundle install Installs a bundle's full component set through each primitive's machinery. The argument may be a catalog bundle id, or a local path to a built `.zip` artifact, a bundle directory, or a `bundle.yml` file; local sources install directly without consulting the catalog stack. -If the current directory is not yet a Spec Kit project, `install` initializes one first so a fresh checkout reaches a working state in a single command. `--integration` selects the integration when initializing a new project, and confirms the target when a bundle pins a specific integration but the project's active integration can't be determined (missing or unreadable `.specify/integration.json`). It does **not** override an already-initialized project's active integration: if a bundle targets a different integration than the project's, install aborts with no changes. Integration-agnostic bundles inherit the project's active integration. Without `--refresh`, installation is idempotent — components already present are skipped. Components installed outside any bundle are skipped and never adopted, so their installed version must match the manifest pin; if it doesn't, or can't be read, install and refresh stop before changing anything and name the component, so you can remove it or install the pinned version yourself. On failure, no provenance record is written (a failed install records nothing), and the components installed during that run are removed on a best-effort basis — removal errors are swallowed, so partial on-disk state may remain. +If the current directory is not yet a Spec Kit project, `install` initializes one first so a fresh checkout reaches a working state in a single command. `--integration` selects the integration when initializing a new project, and confirms the target when a bundle pins a specific integration but the project's active integration can't be determined (missing or unreadable `.specify/integration.json`). It does **not** override an already-initialized project's active integration: if a bundle targets a different integration than the project's, install aborts with no changes. Integration-agnostic bundles inherit the project's active integration. Without `--refresh`, installation is idempotent — already-installed components matching their pins are skipped. A missing or different installed version cannot satisfy a pin. Only `--refresh` can repair a drifted version owned exclusively by this bundle; independently installed components are skipped and never adopted or replaced. On failure, no provenance record is written (a failed install records nothing), and the components installed during that run are removed on a best-effort basis — removal errors are swallowed, so partial on-disk state may remain. + +Installed-bundle records store required component pins separately from contributed components. A compatible independently installed component satisfies a bundle's requirement but remains independently owned: its pin still blocks a later bundle from installing an incompatible version, while removing the bundle does not uninstall it. An unpinned install or refresh cannot replace a component another bundle requires at a pinned version; sharing an already installed matching version without refreshing remains allowed. Shared bundles must also agree on component source and, for presets, priority and strategy before installing or refreshing the shared component. Older records without the requirement list retain their known contributed pins; reinstall or refresh the bundle to record its full requirements. A normal install rejects a change to an already-recorded bundle's version or owned component metadata (version, source, preset priority, or strategy), including removal of an owned component. This applies even if a local manifest keeps the same bundle version. Reordering unchanged components or adding new components does not require refresh. To apply changes to a local bundle without adding it to a catalog, pass the revised source with `--refresh`: @@ -97,9 +99,15 @@ specify bundle update [] Re-resolves a bundle and **refreshes** its components through each primitive's update path, bringing already-installed components up to the bundle's newly pinned versions while preserving primitive-level overrides (such as preset priority). Provide a bundle id, or use `--all` to update everything installed. -**Pinned catalog releases.** An extension or preset pinned to a version other than the one its catalog currently advertises installs that exact release when the winning catalog entry lists it under `releases`, using that release's own download URL and SHA-256 digest. The downloaded archive must declare the pinned ID and version. If the winning catalog entry has no release for the pinned version, install stops with an error rather than substituting the advertised release or falling through to a lower-priority catalog. Workflows and components bundled with Spec Kit still require the pin to match the version they resolve to. +**Pinned catalog releases.** Extensions, presets, workflows, and steps with version pins select that exact release from the highest-priority active catalog entry. Historical releases must be advertised under `releases` with their own artifact URL and SHA-256 digest; the bundler never guesses an old URL from the current one or falls through to another catalog. Legacy extension and preset entries that advertise no version are an exception: the bundler uses the winning entry as-is and cannot enforce its pin or verify the installed archive's version. Publish a versioned entry to make that pin enforceable. The primitive installer uses the selected workflow or step record without re-reading the catalog, and verifies downloaded archive or workflow/step metadata against that release. A pinned workflow that ships with Spec Kit uses its bundled copy when the version matches, and the catalog release when it differs and network access is allowed. A step without a pin installs the current catalog release. + +Without an explicit `source`, bundled extensions and presets take precedence over catalog releases. Validation rejects a pin that differs from the bundled version even when a matching catalog release exists; specify the winning catalog as `source` to opt into its release. + +> **One installed version per component ID.** Bundles sharing a component must agree on its pinned version. A different or unknown pin from another bundle is rejected before installation, including during `bundle update`; refreshing one bundle cannot replace a version required by another. + +A bundle may list a component ID only once per kind; duplicate references in the same manifest are invalid, even if their pins agree. -> **Pin enforcement is install-time only.** Idempotency checks are id-based, not version-aware: a component owned by a bundle that is already present is skipped during `install` without comparing its on-disk version to the manifest pin. Version pins are therefore guaranteed to be applied only when the bundler actually installs a component for the first time or refreshes it. Run `specify bundle update ` for catalog bundles or `specify bundle install --refresh` for local sources to re-apply owned components at their pinned versions. +An optional `source` in a `provides.` reference names the expected winning component catalog (as displayed by `specify catalog list`, or `specify workflow step catalog list` for steps). It is **not** an artifact URL and does not override catalog priority or install policy. When specified, the component is verified against that catalog even if already installed, rather than resolved from a Spec Kit-bundled copy; a different winning catalog or a discovery-only source prevents installation. If multiple active catalogs share the named source, bundle validation and installation reject it as ambiguous; give each catalog a unique name. Verifying an explicit source requires network access. Direct primitive `--from` URLs are a separate, explicitly requested route. ## Remove a Bundle @@ -107,7 +115,7 @@ Re-resolves a bundle and **refreshes** its components through each primitive's u specify bundle remove ``` -Uninstalls only the components this bundle contributed, leaving any component that another installed bundle still needs in place (no collateral removals). +Uninstalls only the components this bundle contributed, leaving any component that another installed bundle still needs in place (no collateral removals). If no other bundle owns a retained component, its contribution is transferred to a remaining requiring bundle so removing that bundle later can clean it up. Independently installed components are never adopted. ## List Installed Bundles @@ -119,7 +127,7 @@ specify bundle list | -------- | ---------------------------- | | `--json` | Emit machine-readable JSON | -Lists the bundles installed in the project with their versions, component counts, and install timestamps. +Lists the bundles installed in the project with their versions, required component counts (including components supplied independently), and install timestamps. ## Initialize a Project with a Bundle @@ -145,7 +153,7 @@ specify bundle validate | `--path` | Bundle directory or `bundle.yml` (default: current directory) | | `--offline` | Verify references against bundled/installed components only | -Reports whether a `bundle.yml` is well-formed and whether every declared component reference resolves. References are checked against bundled components, the project's installed components, and — when online — the active catalogs. Validation fails only when a reference is definitively absent everywhere it could be checked: that is, when an active catalog is reachable and confirms the component is missing. References that cannot be verified — because validation is offline, or because a catalog is unreachable — are downgraded to warnings so authoring can continue, rather than failing the run. +Reports whether a `bundle.yml` is well-formed and whether every declared component reference resolves at its pinned version. References are checked against matching bundled or installed components and — when online — the exact release in the winning install-allowed catalog. An explicit `source` is verified against the winning catalog instead of resolving locally. Missing releases, mismatched sources, discovery-only sources, and malformed catalog metadata fail validation; references that cannot be checked offline or because a catalog is unreachable produce warnings. If a higher-priority catalog is unreachable before a match is found, the lookup remains unverified; a lower-priority match cannot settle it. As with installation, legacy extension and preset entries advertising no version are accepted without verifying their pins. ## Build a Bundle Artifact diff --git a/src/specify_cli/bundles/_commands.py b/src/specify_cli/bundles/_commands.py index 5619ecc04b..66318831d8 100644 --- a/src/specify_cli/bundles/_commands.py +++ b/src/specify_cli/bundles/_commands.py @@ -136,7 +136,7 @@ def _bundle_overlaps(project_root: Path, manifest, *, offline: bool) -> list[str active_integration(project_root), load_records(project_root), ) - return list(report.overlaps) + return [*report.overlaps, *report.version_clashes] except BundlerError: return [] diff --git a/src/specify_cli/bundles/adapters.py b/src/specify_cli/bundles/adapters.py index ea3739c576..5427f01255 100644 --- a/src/specify_cli/bundles/adapters.py +++ b/src/specify_cli/bundles/adapters.py @@ -22,9 +22,9 @@ from .._assets import _locate_core_pack, _repo_root from .._download_security import MAX_JSON_CATALOG_BYTES, read_response_limited from . import BundlerError -from .yamlio import loads_json from .catalogs import CatalogSource from .manifest import ComponentRef +from .yamlio import loads_json COMMUNITY_CATALOG_URL = ( "https://raw.githubusercontent.com/github/spec-kit/main/" @@ -330,6 +330,30 @@ def installed_version( manager = self._manager_for(component, project_root) return manager.installed_version(component) + def validate_source(self, project_root: Path, component: ComponentRef) -> None: + if not self._allow_network: + raise BundlerError( + f"Cannot verify catalog source '{component.source}' for " + f"{component.kind[:-1]} '{component.id}' offline; " + "re-run without --offline." + ) + from .references import _resolved_in_catalog + + result = _resolved_in_catalog(project_root, component) + if result is True: + return + if result is None: + detail = "catalog unreachable" + elif result is False: + detail = "the winning install-allowed catalog has no matching release" + else: + detail = result + raise BundlerError( + f"Cannot verify {component.kind[:-1]} '{component.id}' at " + f"{component.version or 'current'} from catalog " + f"'{component.source}': {detail}." + ) + def install(self, project_root: Path, component: ComponentRef) -> None: manager = self._manager_for(component, project_root) manager.install(component) diff --git a/src/specify_cli/bundles/command_list.py b/src/specify_cli/bundles/command_list.py index 42acea2c38..e6b4da6817 100644 --- a/src/specify_cli/bundles/command_list.py +++ b/src/specify_cli/bundles/command_list.py @@ -40,6 +40,6 @@ def bundle_list( console.print( f" [bold]{_escape_markup(str(record.bundle_id))}[/bold] " f"v{_escape_markup(str(record.version))} " - f"[dim]({len(record.contributed_components)} components, " + f"[dim]({len(record.required_components)} components, " f"installed {_escape_markup(str(record.installed_at))})[/dim]" ) diff --git a/src/specify_cli/bundles/component_catalog.py b/src/specify_cli/bundles/component_catalog.py new file mode 100644 index 0000000000..f52413c745 --- /dev/null +++ b/src/specify_cli/bundles/component_catalog.py @@ -0,0 +1,149 @@ +"""Resolve bundle components from the first trustworthy catalog source.""" + +from __future__ import annotations + +import ssl +from http.client import HTTPException +from urllib.error import HTTPError, URLError + +from ..authentication.http import RedirectPolicyError +from . import BundlerError +from .manifest import ComponentRef + + +class CatalogUnavailable(BundlerError): + """The winning component source cannot be determined right now.""" + + +def _is_unavailable(exc: BaseException) -> bool: + seen: set[int] = set() + while exc is not None and id(exc) not in seen: + seen.add(id(exc)) + if isinstance(exc, RedirectPolicyError): + return False + if isinstance(exc, HTTPError): + return exc.code in (408, 429) or 500 <= exc.code <= 599 + if isinstance(exc, URLError): + return not isinstance(exc.reason, (ssl.SSLError, ValueError)) + if isinstance(exc, (ConnectionError, TimeoutError, HTTPException)): + return True + exc = exc.__cause__ or exc.__context__ + return False + + +def _source_entries(data: object, component: ComponentRef, url: str) -> dict | list: + entries = data.get(component.kind) if isinstance(data, dict) else None + if not isinstance(entries, (dict, list)): + raise BundlerError( + f"Invalid {component.kind[:-1]} catalog from {url}: " + f"missing or malformed {component.kind} metadata." + ) + return entries + + +def _matching_entry(entries: dict | list, component: ComponentRef, url: str) -> dict | None: + if isinstance(entries, dict): + if component.id not in entries: + return None + found = entries[component.id] + if not isinstance(found, dict): + raise BundlerError( + f"Invalid {component.kind[:-1]} catalog entry for '{component.id}' " + f"from {url}: expected an object." + ) + return {**found, "id": component.id} + + seen: set[str] = set() + found = None + for item in entries: + if not isinstance(item, dict): + raise BundlerError( + f"Invalid {component.kind[:-1]} catalog entry from {url}: " + "expected an object." + ) + item_id = item.get("id") + if not isinstance(item_id, str): + raise BundlerError( + f"Invalid {component.kind[:-1]} ID in catalog {url}: expected a string." + ) + item_id = item_id.strip() if component.kind == "steps" else item_id + if not item_id: + raise BundlerError( + f"Invalid {component.kind[:-1]} ID in catalog {url}: empty ID." + ) + if item_id in seen: + raise BundlerError( + f"Duplicate {component.kind[:-1]} ID '{item_id}' in catalog {url}." + ) + seen.add(item_id) + if item_id == component.id: + found = {**item, "id": item_id} + return found + + +def winning_catalog_entry(catalog, component: ComponentRef) -> dict | None: + """Read sources in priority order rather than accepting a lower match on error. + + The component catalogs' public ID lookup skips failed catalogs. Bundles + cannot do that: an unreadable higher source may own the requested ID. + Reuse each catalog's authenticated, cached, redirect-checked fetcher and + its existing release selector; only the strict winner policy lives here. + """ + from ..extensions import ExtensionError + from ..presets import PresetError + from ..workflows.catalog import StepCatalogError, WorkflowCatalogError + + sources = catalog.get_active_catalogs() + if component.source and sum( + source.name == component.source for source in sources + ) > 1: + raise BundlerError( + f"{component.kind[:-1]} '{component.id}' requests ambiguous catalog " + f"source '{component.source}': multiple active catalogs share this name. " + "Give each catalog a unique name before installing." + ) + + for source in sources: + try: + data = catalog._fetch_single_catalog(source) + except ( + ExtensionError, PresetError, WorkflowCatalogError, StepCatalogError, + OSError, HTTPException, UnicodeError, ValueError, TypeError, + RecursionError, + ) as exc: + if _is_unavailable(exc): + raise CatalogUnavailable( + f"Catalog '{source.name}' is unreachable: {exc}" + ) from exc + raise BundlerError( + f"Invalid {component.kind[:-1]} catalog from {source.url}: {exc}" + ) from exc + + entries = _source_entries(data, component, source.url) + found = _matching_entry(entries, component, source.url) + if found is not None: + return { + **found, + "_catalog_name": source.name, + "_install_allowed": source.install_allowed, + } + return None + + +def select_catalog_release(component: ComponentRef, entry: dict) -> dict | None: + """Select from the already-fetched winning entry without a second lookup.""" + if component.kind == "extensions": + from ..extensions._catalog_versions import select_release + + return select_release(entry, component.version) + if component.kind == "presets": + from ..presets._catalog_versions import select_release + + return select_release(entry, component.version) + if component.kind == "workflows": + from ..workflows.catalog._versions import select_release + + return select_release(entry, component.version) + from ..workflows.step.catalog._versions import select_release + + return select_release(entry, component.id, component.version) diff --git a/src/specify_cli/bundles/conflict.py b/src/specify_cli/bundles/conflict.py index 78938c5887..db79614486 100644 --- a/src/specify_cli/bundles/conflict.py +++ b/src/specify_cli/bundles/conflict.py @@ -1,27 +1,22 @@ -"""Conflict detection across the installed-bundle stack. - -The single cross-bundle conflict point is the active integration (FR-019). -Component-level overlaps (same preset id at different priorities, etc.) are -resolved by the existing primitive machinery's own precedence rules, so the -bundler only needs to guard the integration invariant and surface informational -overlaps. -""" +"""Conflict detection across the installed-bundle stack.""" from __future__ import annotations from dataclasses import dataclass, field from .manifest import BundleManifest from .records import InstalledBundleRecord +from .versioning import same_version @dataclass class ConflictReport: integration_clash: str | None = None # message when a hard clash exists - overlaps: list[str] = field(default_factory=list) # components already provided + version_clashes: list[str] = field(default_factory=list) + overlaps: list[str] = field(default_factory=list) # components already required @property def has_blocking_conflict(self) -> bool: - return self.integration_clash is not None + return self.integration_clash is not None or bool(self.version_clashes) def detect_conflicts( @@ -31,24 +26,40 @@ def detect_conflicts( ) -> ConflictReport: report = ConflictReport() - if manifest.integration is not None and active_integration: - if manifest.integration.id != active_integration: - report.integration_clash = ( - f"Bundle targets integration '{manifest.integration.id}' but the " - f"project's active integration is '{active_integration}'." - ) + if ( + manifest.integration is not None + and active_integration + and manifest.integration.id != active_integration + ): + report.integration_clash = ( + f"Bundle targets integration '{manifest.integration.id}' but the " + f"project's active integration is '{active_integration}'." + ) - already: dict[tuple[str, str], str] = {} + already: dict[tuple[str, str], list[tuple[str, str | None]]] = {} for record in installed: - for component in record.contributed_components: - already[(component.kind, component.id)] = record.bundle_id + for component in record.required_components: + already.setdefault((component.kind, component.id), []).append( + (record.bundle_id, component.version) + ) for component in manifest.components: - owner = already.get((component.kind, component.id)) - if owner and owner != manifest.bundle.id: - report.overlaps.append( - f"{component.kind[:-1]} '{component.id}' is already provided by " - f"bundle '{owner}'." - ) + for owner, version in already.get((component.kind, component.id), []): + if owner == manifest.bundle.id: + continue + if component.version and ( + not version or not same_version(component.version, version) + ): + report.version_clashes.append( + f"{component.kind[:-1]} '{component.id}' requires version " + f"{component.version}, but bundle '{owner}' already requires " + f"version {version or ''}. Only one version can be " + "installed per ID." + ) + else: + report.overlaps.append( + f"{component.kind[:-1]} '{component.id}' is already required by " + f"bundle '{owner}'." + ) return report diff --git a/src/specify_cli/bundles/installer.py b/src/specify_cli/bundles/installer.py index 29c7cfae61..23fcf1e94c 100644 --- a/src/specify_cli/bundles/installer.py +++ b/src/specify_cli/bundles/installer.py @@ -16,6 +16,7 @@ from typing import Protocol from . import BundlerError +from .conflict import detect_conflicts from .manifest import BundleManifest, ComponentRef from .records import ( InstalledBundleRecord, @@ -24,9 +25,9 @@ load_records, remove_record, save_records, + transfer_contributions, upsert_record, ) -from .conflict import detect_conflicts from .resolver import InstallPlan from .versioning import same_version @@ -36,6 +37,12 @@ class PrimitiveInstaller(Protocol): def is_installed(self, project_root: Path, component: ComponentRef) -> bool: ... + def installed_version( + self, project_root: Path, component: ComponentRef + ) -> str | None: ... + + def validate_source(self, project_root: Path, component: ComponentRef) -> None: ... + def install(self, project_root: Path, component: ComponentRef) -> None: ... def remove(self, project_root: Path, component: ComponentRef) -> None: ... @@ -78,26 +85,24 @@ def install_bundle( config (e.g. preset priority overrides) is preserved by the underlying machinery. - Version-pin enforcement is install-time only. The primitive ``is_installed`` - checks are id-based (they do not compare versions), so when a component is - already present and *refresh* is False it is skipped without verifying that - the on-disk version matches the manifest pin. Changes to a recorded bundle's - version or owned component metadata, including removals, are rejected unless - *refresh* is True, preventing stale or orphaned components. Pins are only - guaranteed to be applied when the bundler actually performs an install or a - refresh; running ``specify bundle update`` re-applies every owned component - at its pinned version. - - The exception is a component installed independently of any bundle: it is - never installed or refreshed here, so its installed version must already - match the pin, or the call fails before changing anything. + Already-installed components must match their pins before they can be + skipped. A refresh may repair drift in components owned exclusively by + this bundle, but cannot change a version required by another bundle or an + independently installed component. Changes to owned component metadata + still require refresh. Other bundles' source and preset settings cannot be + changed by sharing or refreshing a component. """ records = load_records(project_root) if manifest is not None: report = detect_conflicts(manifest, plan.effective_integration, records) if report.has_blocking_conflict: - raise BundlerError(report.integration_clash) + raise BundlerError( + "; ".join( + [*([report.integration_clash] if report.integration_clash else []), + *report.version_clashes] + ) + ) result = InstallResult(bundle_id=plan.bundle_id) existing = find_record(records, plan.bundle_id) @@ -132,10 +137,23 @@ def install_bundle( if r.bundle_id != plan.bundle_id for c in r.contributed_components } + other_pins: dict[tuple[str, str], set[str]] = {} + other_requirements: dict[tuple[str, str], list[tuple[str, ComponentRef]]] = {} + for record in records: + if record.bundle_id == plan.bundle_id: + continue + for component in record.required_components: + key = component.kind, component.id + other_requirements.setdefault(key, []).append((record.bundle_id, component)) + if component.version: + other_pins.setdefault(key, set()).add(component.version) contributed: list[ComponentRef] = [] done: list[ComponentRef] = [] try: - _check_unowned_pins(project_root, plan, installer, prior_ours | other_tracked) + _check_installed_pins( + project_root, plan, installer, prior_ours, other_tracked, + other_pins, other_requirements, refresh=refresh, + ) for component in plan.components: key = (component.kind, component.id) if installer.is_installed(project_root, component): @@ -182,7 +200,7 @@ def install_bundle( except BundlerError: _rollback(project_root, installer, done) raise - except Exception as exc: # noqa: BLE001 + except Exception as exc: _rollback(project_root, installer, done) raise BundlerError( f"Failed to install bundle '{plan.bundle_id}': {exc}. " @@ -193,11 +211,22 @@ def install_bundle( bundle_id=plan.bundle_id, version=plan.version, components=contributed, + required_components=plan.components, # Preserve the original install time across refresh/update so # ``bundle list`` keeps reporting when the bundle was first installed. installed_at=existing.installed_at if existing is not None else None, ) - save_records(project_root, upsert_record(records, record)) + updated = upsert_record(records, record) + if refresh and existing is not None: + planned = {(c.kind, c.id) for c in plan.components} + updated = transfer_contributions( + updated, + [ + component for component in existing.contributed_components + if (component.kind, component.id) not in planned + ], + ) + save_records(project_root, updated) return result @@ -226,8 +255,11 @@ def remove_bundle( remove_attempted = True installer.remove(project_root, component) result.uninstalled.append(component) - save_records(project_root, remove_record(records, bundle_id)) - except Exception as exc: # noqa: BLE001 + remaining = transfer_contributions( + remove_record(records, bundle_id), target.contributed_components + ) + save_records(project_root, remaining) + except Exception as exc: if result.uninstalled: detail = ( f"{len(result.uninstalled)} component(s) were already removed " @@ -252,41 +284,82 @@ def remove_bundle( return result -def _check_unowned_pins( +def _check_installed_pins( project_root: Path, plan: InstallPlan, installer: PrimitiveInstaller, - owned: set[tuple[str, str]], + prior_ours: set[tuple[str, str]], + other_tracked: set[tuple[str, str]], + other_pins: dict[tuple[str, str], set[str]], + other_requirements: dict[tuple[str, str], list[tuple[str, ComponentRef]]], + *, + refresh: bool, ) -> None: - """Refuse to skip an independently installed component that misses its pin. - - A component tracked by no bundle is skipped and never refreshed (FR-022), so - skipping it is only correct when it already has the pinned version. - Otherwise the bundle record would advance while the project keeps running a - different version (#4434). Runs before any primitive is touched. A component - whose installed version can't be read fails too, since it can't be shown to - match. Installers without an ``installed_version`` hook are not checked. - """ - installed_version = getattr(installer, "installed_version", None) - if not callable(installed_version): - return + """Check installed pins and other bundles' requirements before mutation.""" mismatches = [] for component in plan.components: - if not component.version or (component.kind, component.id) in owned: + key = component.kind, component.id + for bundle_id, required in other_requirements.get(key, []): + different = [] + if component.source != required.source: + different.append("source") + if component.kind == "presets": + if component.priority != required.priority: + different.append("priority") + if component.strategy != required.strategy: + different.append("strategy") + if different: + raise BundlerError( + f"Cannot install or refresh shared {component.kind[:-1]} " + f"'{component.id}': bundle '{bundle_id}' requires different " + f"{', '.join(different)}. Shared components must agree on " + "install-affecting requirements." + ) + if component.source: + installer.validate_source(project_root, component) + if refresh and not component.version and key in other_tracked: + raise BundlerError( + f"Cannot refresh unpinned shared {component.kind[:-1]} " + f"'{component.id}': another bundle may require its installed " + "version. Pin this component before refreshing." + ) + if not component.version: + pins = other_pins.get(key) + if pins: + installed = installer.is_installed(project_root, component) + actual = ( + installer.installed_version(project_root, component) + if installed else None + ) + if ( + (refresh and key in prior_ours) + or not actual + or any(not same_version(actual, pin) for pin in pins) + ): + raise BundlerError( + f"Cannot install or refresh unpinned {component.kind[:-1]} " + f"'{component.id}': another bundle requires version " + f"{', '.join(sorted(pins))}. Pin this component to a " + "compatible version before installing or refreshing." + ) continue if not installer.is_installed(project_root, component): continue - actual = installed_version(project_root, component) + actual = installer.installed_version(project_root, component) + if actual and same_version(actual, component.version): + continue + if refresh and key in prior_ours and key not in other_tracked: + continue pinned = f"{component.kind[:-1]} '{component.id}' to {component.version}" - if actual is None: + if not actual: mismatches.append(f"{pinned}, but its installed version is unknown") - elif not same_version(actual, component.version): + else: mismatches.append(f"{pinned}, but {actual} is installed") if mismatches: raise BundlerError( - f"Bundle '{plan.bundle_id}' pins {'; '.join(mismatches)}. Bundles " - "leave components installed outside any bundle unchanged, so remove " - "the installed version or install the pinned one yourself, then re-run." + f"Bundle '{plan.bundle_id}' pins {'; '.join(mismatches)}. Only one " + "version can be installed per ID; independently installed and " + "other bundles' components cannot be replaced by this bundle." ) @@ -315,5 +388,5 @@ def _rollback( for component in reversed(done): try: installer.remove(project_root, component) - except Exception: # noqa: BLE001 - best-effort rollback + except Exception: # noqa: BLE001, S112 - rollback cannot mask the original failure continue diff --git a/src/specify_cli/bundles/manifest.py b/src/specify_cli/bundles/manifest.py index 71304bc682..179abebd12 100644 --- a/src/specify_cli/bundles/manifest.py +++ b/src/specify_cli/bundles/manifest.py @@ -85,14 +85,14 @@ def components(self) -> list[ComponentRef]: # -- construction --------------------------------------------------------- @classmethod - def from_file(cls, path: Path) -> "BundleManifest": + def from_file(cls, path: Path) -> BundleManifest: data = load_yaml(path) manifest = cls.from_dict(data) manifest.source_path = Path(path) return manifest @classmethod - def from_dict(cls, data: Any) -> "BundleManifest": + def from_dict(cls, data: Any) -> BundleManifest: if not isinstance(data, dict): raise BundlerError("Manifest must be a YAML mapping at the top level.") @@ -192,9 +192,17 @@ def structural_errors(self) -> list[str]: "(lowercase letters, digits, '.', '_', '-'; no path separators)." ) + seen_components: set[tuple[str, str]] = set() for ref in self.components: if not ref.id: errors.append(f"A {ref.kind[:-1]} entry is missing its 'id'.") + elif (ref.kind, ref.id) in seen_components: + errors.append( + f"Duplicate {ref.kind[:-1]} '{ref.id}' in provides; " + "declare each component ID only once per kind." + ) + else: + seen_components.add((ref.kind, ref.id)) if ref.kind != "steps" and not ref.version: errors.append( f"{ref.kind[:-1]} '{ref.id or ''}' must be pinned to a 'version'." diff --git a/src/specify_cli/bundles/primitives.py b/src/specify_cli/bundles/primitives.py index 26a0518d44..bb37955ab0 100644 --- a/src/specify_cli/bundles/primitives.py +++ b/src/specify_cli/bundles/primitives.py @@ -26,6 +26,7 @@ from . import BundlerError from .manifest import ComponentRef +from .versioning import same_version DEFAULT_PRIORITY = 10 @@ -68,6 +69,7 @@ def _select_pinned_release( lower-priority catalog. An entry advertising no version cannot enforce the pin, so it is installed as resolved (mirrors ``_assert_pinned_version``). """ + _assert_catalog_source(kind, component, info) pinned = component.version advertised = info.get("version") if not pinned or advertised is None or not str(advertised).strip(): @@ -83,6 +85,53 @@ def _select_pinned_release( return selected, selected["version"] +def _assert_catalog_source(kind: str, component: ComponentRef, info: dict) -> None: + """A source identifies the winning catalog; it cannot select another one.""" + if component.source and component.source != info.get("_catalog_name"): + raise BundlerError( + f"{kind} '{component.id}' requests catalog '{component.source}', " + f"but its highest-priority source is " + f"'{info.get('_catalog_name', '')}'. " + "A bundle source cannot bypass catalog precedence." + ) + if not info.get("_install_allowed", True): + raise BundlerError( + f"{kind} '{component.id}' is from a discovery-only catalog; " + "installation is not allowed." + ) + + +def _selected_catalog_info(kind: str, component: ComponentRef, catalog) -> dict: + """Resolve an exact workflow/step release within the winning catalog.""" + from .component_catalog import select_catalog_release, winning_catalog_entry + + current = winning_catalog_entry(catalog, component) + if current is None: + raise BundlerError(f"{kind} '{component.id}' not found in any catalog.") + _assert_catalog_source(kind, component, current) + if component.version is None: + return current + selected = select_catalog_release(component, current) + if selected is None: + raise BundlerError( + f"{kind} '{component.id}' has no catalog release for pinned version " + f"{component.version} in the highest-priority source." + ) + if selected.get("_catalog_name") != current.get("_catalog_name"): + raise BundlerError( + f"{kind} '{component.id}' changed catalog sources during version lookup." + ) + _assert_catalog_source(kind, component, selected) + if not selected.get("version") or not same_version( + selected["version"], component.version + ): + raise BundlerError( + f"{kind} '{component.id}' has no verifiable catalog release for " + f"pinned version {component.version}." + ) + return selected + + def _bundled_manifest_version(manifest_path: Path, root_key: str) -> str | None: """Best-effort read of a bundled asset's declared version from its manifest. @@ -102,7 +151,7 @@ def _bundled_manifest_version(manifest_path: Path, root_key: str) -> str | None: # (missing / non-string / whitespace) means "cannot enforce". if isinstance(version, str) and version.strip(): return version - except Exception: # noqa: BLE001 - unreadable/invalid manifest: skip pin + except Exception: # noqa: BLE001 - unreadable manifest: version unknown return None return None @@ -213,7 +262,7 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: priority = DEFAULT_PRIORITY if component.priority is None else component.priority bundled = _locate_bundled_preset(component.id) - if bundled is not None: + if bundled is not None and component.source is None: # Enforce the manifest pin against the bundled asset's own version, # mirroring the catalog path below (the bundled path previously # skipped the pin entirely). @@ -236,16 +285,12 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: ) from ..presets import PresetCatalog + from .component_catalog import winning_catalog_entry catalog = PresetCatalog(self._root) - info = catalog.get_pack_info(component.id) + info = winning_catalog_entry(catalog, component) if not info: raise BundlerError(f"Preset '{component.id}' not found in any catalog.") - if not info.get("_install_allowed", True): - raise BundlerError( - f"Preset '{component.id}' is from a discovery-only catalog; " - "installation is not allowed." - ) from ..presets._catalog_versions import select_release info, expected_version = _select_pinned_release( @@ -273,7 +318,7 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: def remove(self, component: ComponentRef) -> None: try: self._manager.remove(component.id) - except Exception as exc: # noqa: BLE001 + except Exception as exc: raise BundlerError( f"Failed to remove preset '{component.id}': {exc}" ) from exc @@ -310,7 +355,7 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: priority = DEFAULT_PRIORITY if component.priority is None else component.priority bundled = _locate_bundled_extension(component.id) - if bundled is not None: + if bundled is not None and component.source is None: # Enforce the manifest pin against the bundled asset's own version, # mirroring the catalog path below (the bundled path previously # skipped the pin entirely). @@ -334,18 +379,14 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: ) from ..extensions import ExtensionCatalog + from .component_catalog import winning_catalog_entry catalog = ExtensionCatalog(self._root) - info = catalog.get_extension_info(component.id) + info = winning_catalog_entry(catalog, component) if not info: raise BundlerError( f"Extension '{component.id}' not found in any catalog." ) - if not info.get("_install_allowed", True): - raise BundlerError( - f"Extension '{component.id}' is from a discovery-only catalog; " - "installation is not allowed." - ) from ..extensions._catalog_versions import select_release info, expected_version = _select_pinned_release( @@ -374,7 +415,7 @@ def _do_install(self, component: ComponentRef, *, force: bool) -> None: def remove(self, component: ComponentRef) -> None: try: self._manager.remove(component.id) - except Exception as exc: # noqa: BLE001 + except Exception as exc: raise BundlerError( f"Failed to remove extension '{component.id}': {exc}" ) from exc @@ -401,7 +442,7 @@ def install(self, component: ComponentRef) -> None: from .._assets import _locate_bundled_workflow bundled = _locate_bundled_workflow(component.id) - if bundled is not None: + if bundled is not None and component.source is None: workflow_file = bundled / "workflow.yml" try: from ..workflows.engine import WorkflowDefinition @@ -416,18 +457,22 @@ def install(self, component: ComponentRef) -> None: f"Bundled workflow at {workflow_file} declares ID " f"'{definition.id}', expected '{component.id}'." ) - _assert_pinned_version( - "Workflow", component.id, component.version, definition.version - ) - from .. import workflow_add - - with _chdir(self._root): - _delegate_command( - "install", - f"workflow '{component.id}'", - lambda: workflow_add(str(workflow_file), dev=True, from_url=None), + if component.version is None or ( + definition.version and same_version(definition.version, component.version) + ): + from .. import workflow_add + + with _chdir(self._root): + _delegate_command( + "install", + f"workflow '{component.id}'", + lambda: workflow_add(str(workflow_file), dev=True, from_url=None), + ) + return + if not self._allow_network: + _assert_pinned_version( + "Workflow", component.id, component.version, definition.version ) - return if not self._allow_network: raise BundlerError( @@ -435,13 +480,20 @@ def install(self, component: ComponentRef) -> None: "access is disabled. Installing or refreshing this component " "requires network access; re-run without --offline." ) - self._assert_pinned_version(component) - from .. import workflow_add + from ..workflows.catalog import WorkflowCatalog + + catalog = WorkflowCatalog(self._root) + selected = _selected_catalog_info( + "Workflow", component, catalog + ) + from ..workflows.command_add import _install_preselected_workflow with _chdir(self._root): _delegate_command( "install", f"workflow '{component.id}'", - lambda: workflow_add(component.id, dev=False, from_url=None), + lambda: _install_preselected_workflow( + component.id, version=component.version, selected_info=selected, + ), ) def refresh(self, component: ComponentRef) -> None: @@ -449,20 +501,6 @@ def refresh(self, component: ComponentRef) -> None: # to the standard install path which handles version refresh correctly. self.install(component) - def _assert_pinned_version(self, component: ComponentRef) -> None: - if not component.version: - return - try: - from ..workflows.catalog import WorkflowCatalog - - info = WorkflowCatalog(self._root).get_workflow_info(component.id) - except Exception: # noqa: BLE001 - catalog unreachable: cannot enforce - return - if info: - _assert_pinned_version( - "Workflow", component.id, component.version, info.get("version") - ) - def remove(self, component: ComponentRef) -> None: from .. import workflow_remove @@ -497,13 +535,39 @@ def install(self, component: ComponentRef) -> None: "is disabled. Installing or refreshing this component requires " "network access; re-run without --offline." ) - from .. import workflow_step_add + from ..workflows.catalog import StepCatalog, StepCatalogError + from ..workflows.step.installer import StepInstallError, validate_step_id - with _chdir(self._root): - _delegate_command( - "install", f"step '{component.id}'", - lambda: workflow_step_add(component.id), + try: + validate_step_id(component.id) + except StepInstallError as exc: + raise BundlerError( + f"Invalid step '{component.id}': {exc}" + ) from exc + + try: + catalog = StepCatalog(self._root) + selected = _selected_catalog_info( + "Step", component, catalog ) + except StepCatalogError as exc: + raise BundlerError( + f"Failed to resolve step '{component.id}': {exc}" + ) from exc + from ..workflows.step.command_add import _install_preselected_step + + with _chdir(self._root): + try: + _delegate_command( + "install", f"step '{component.id}'", + lambda: _install_preselected_step( + component.id, version=component.version, selected_info=selected, + ), + ) + except StepInstallError as exc: + raise BundlerError( + f"Failed to install step '{component.id}': {exc}" + ) from exc def refresh(self, component: ComponentRef) -> None: # Preserve an existing step until we've validated we can perform refresh. diff --git a/src/specify_cli/bundles/records.py b/src/specify_cli/bundles/records.py index 1b0628833e..c3a616cd93 100644 --- a/src/specify_cli/bundles/records.py +++ b/src/specify_cli/bundles/records.py @@ -1,12 +1,13 @@ -"""Installed-bundle records — provenance for precise list/remove/update. +"""Installed-bundle records — requirements and ownership for list/remove/update. Records are stored as JSON at ``.specify/bundle-records.json``. Each record -captures exactly which components a bundle contributed so removal touches only -that bundle's components and never collateral (FR-022, SC-004). +stores every required component for conflict detection and separately records +which components the bundle contributed, so removal never touches independently +installed components (FR-022, SC-004). """ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, replace from datetime import datetime, timezone from pathlib import Path from typing import Any @@ -24,6 +25,7 @@ class InstalledBundleRecord: bundle_id: str version: str contributed_components: tuple[ComponentRef, ...] + required_components: tuple[ComponentRef, ...] installed_at: str @classmethod @@ -33,11 +35,15 @@ def create( version: str, components: list[ComponentRef], installed_at: str | None = None, + required_components: list[ComponentRef] | None = None, ) -> "InstalledBundleRecord": return cls( bundle_id=bundle_id, version=version, contributed_components=tuple(components), + required_components=tuple( + components if required_components is None else required_components + ), installed_at=installed_at or _utc_now(), ) @@ -49,6 +55,9 @@ def to_dict(self) -> dict[str, Any]: "contributed_components": [ _component_to_dict(c) for c in self.contributed_components ], + "required_components": [ + _component_to_dict(c) for c in self.required_components + ], } @classmethod @@ -65,6 +74,13 @@ def from_dict(cls, data: Any) -> "InstalledBundleRecord": raise BundlerError( "Corrupt record: 'contributed_components' must be a list." ) + # Older records contain only contributed components. Retain those + # known pins until a successful reinstall writes the full requirements. + required_raw = data.get("required_components", components_raw) + if not isinstance(required_raw, list): + raise BundlerError( + "Corrupt record: 'required_components' must be a list." + ) # ``.get(key, "")`` defaults only a *missing* key. A key that is # present but null -- how a hand-edited or corrupt record spells an # empty field -- yields ``None``, and ``str(None)`` is the non-empty @@ -83,13 +99,21 @@ def from_dict(cls, data: Any) -> "InstalledBundleRecord": f"Corrupt records file: record for bundle '{bundle_id}' is " "missing its 'version'." ) + contributed = tuple(_component_from_dict(c) for c in components_raw) + required = tuple( + _component_from_dict(c, label="required") for c in required_raw + ) + if not set(contributed).issubset(required): + raise BundlerError( + "Corrupt record: 'required_components' must include all " + "'contributed_components'." + ) return cls( bundle_id=bundle_id, version=version, installed_at=_text(data.get("installed_at")), - contributed_components=tuple( - _component_from_dict(c) for c in components_raw - ), + contributed_components=contributed, + required_components=required, ) @@ -178,6 +202,39 @@ def remove_record( return [r for r in records if r.bundle_id != bundle_id] +def transfer_contributions( + records: list[InstalledBundleRecord], + released: list[ComponentRef] | tuple[ComponentRef, ...], +) -> list[InstalledBundleRecord]: + """Keep a bundle-installed component attributed while another bundle needs it.""" + updated = list(records) + owned = { + (component.kind, component.id) + for record in updated + for component in record.contributed_components + } + for component in released: + key = component.kind, component.id + if key in owned: + continue + for index, record in enumerate(updated): + required = next( + ( + ref for ref in record.required_components + if (ref.kind, ref.id) == key + ), + None, + ) + if required is not None: + updated[index] = replace( + record, + contributed_components=(*record.contributed_components, required), + ) + owned.add(key) + break + return updated + + def components_still_needed( records: list[InstalledBundleRecord], exclude_bundle_id: str ) -> set[tuple[str, str]]: @@ -186,7 +243,7 @@ def components_still_needed( for record in records: if record.bundle_id == exclude_bundle_id: continue - for component in record.contributed_components: + for component in record.required_components: needed.add((component.kind, component.id)) return needed @@ -204,9 +261,9 @@ def _component_to_dict(ref: ComponentRef) -> dict[str, Any]: return data -def _component_from_dict(data: Any) -> ComponentRef: +def _component_from_dict(data: Any, *, label: str = "contributed") -> ComponentRef: if not isinstance(data, dict): - raise BundlerError("Each contributed component must be a mapping.") + raise BundlerError(f"Each {label} component must be a mapping.") kind = _text(data.get("kind")) cid = _text(data.get("id")) if kind not in COMPONENT_KINDS: @@ -216,7 +273,7 @@ def _component_from_dict(data: Any) -> ComponentRef: ) if not cid: raise BundlerError( - "Corrupt records file: a contributed component is missing its 'id'." + f"Corrupt records file: a {label} component is missing its 'id'." ) return ComponentRef( kind=kind, diff --git a/src/specify_cli/bundles/references.py b/src/specify_cli/bundles/references.py index 822b3d991a..5f4eab9f88 100644 --- a/src/specify_cli/bundles/references.py +++ b/src/specify_cli/bundles/references.py @@ -13,35 +13,48 @@ from pathlib import Path from .manifest import ComponentRef +from .versioning import same_version + + +def _matches_pin(component: ComponentRef, actual: str | None) -> bool: + return component.version is None or ( + bool(actual) and same_version(actual, component.version) + ) def _resolved_locally(root: Path, component: ComponentRef) -> bool: kind = component.kind try: + if component.source: + return False + from .primitives import _bundled_manifest_version, primitive_manager + if kind == "presets": from .._assets import _locate_bundled_preset - from ..presets import PresetManager - if _locate_bundled_preset(component.id) is not None: + bundled = _locate_bundled_preset(component.id) + if bundled is not None and _matches_pin( + component, _bundled_manifest_version(bundled / "preset.yml", "preset") + ): return True - return PresetManager(root).get_pack(component.id) is not None if kind == "extensions": from .._assets import _locate_bundled_extension - from ..extensions import ExtensionManager - if _locate_bundled_extension(component.id) is not None: + bundled = _locate_bundled_extension(component.id) + if bundled is not None and _matches_pin( + component, _bundled_manifest_version(bundled / "extension.yml", "extension") + ): return True - return ExtensionManager(root).registry.is_installed(component.id) if kind == "workflows": from .._assets import _locate_bundled_workflow - from ..workflows.catalog import WorkflowRegistry - if _locate_bundled_workflow(component.id) is not None: + bundled = _locate_bundled_workflow(component.id) + if bundled is not None and _matches_pin( + component, _bundled_manifest_version(bundled / "workflow.yml", "workflow") + ): return True - return WorkflowRegistry(root).is_installed(component.id) if kind == "steps": from ..workflows import BUILTIN_STEP_TYPES - from ..workflows.catalog import StepRegistry # Step types ship with Spec Kit as built-ins (shell, gate, if, ...) # rather than as an on-disk asset directory, so there is no @@ -53,36 +66,146 @@ def _resolved_locally(root: Path, component: ComponentRef) -> bool: # loaded for one project would be accepted as "bundled" when # validating another. Without any bundled check at all, every # built-in step type looked unresolved. - if component.id in BUILTIN_STEP_TYPES: + if component.id in BUILTIN_STEP_TYPES and component.version is None: return True - return StepRegistry(root).is_installed(component.id) + manager = primitive_manager(kind, root, allow_network=False) + return manager.is_installed(component) and _matches_pin( + component, manager.installed_version(component) + ) except Exception: # noqa: BLE001 - resolution is best-effort return False return False -def _resolved_in_catalog(root: Path, component: ComponentRef) -> bool | None: - """Return True/False if a catalog could be consulted, or None on failure.""" +def _validate_pinned_install_metadata(component: ComponentRef, selected: dict) -> None: + from .._download_security import is_https_or_localhost_http + from . import BundlerError + + def require_url(value: object, label: str) -> str: + if not isinstance(value, str) or not is_https_or_localhost_http(value): + raise BundlerError( + f"{component.kind[:-1]} '{component.id}' has an invalid {label} " + f"for pinned version {component.version}." + ) + return value + + if component.kind in ("extensions", "presets"): + from ..shared_infra import _SHA256_HEX_RE + + require_url(selected.get("download_url"), "download URL") + digest = selected.get("sha256") + if digest is not None: + value = str(digest).strip() + if value[:7].lower() == "sha256:": + value = value[7:].strip() + if not _SHA256_HEX_RE.fullmatch(value.lower()): + raise BundlerError( + f"{component.kind[:-1]} '{component.id}' has an invalid SHA-256 " + f"digest for pinned version {component.version}." + ) + return + + if component.kind == "workflows": + from ..workflows.catalog._versions import _SHA256 + + require_url(selected.get("url"), "install URL") + digest = selected.get("sha256") + if digest is not None and ( + not isinstance(digest, str) or not _SHA256.fullmatch(digest) + ): + raise BundlerError( + f"workflow '{component.id}' has an invalid SHA-256 digest " + f"for pinned version {component.version}." + ) + if "requires" in selected and not isinstance(selected["requires"], dict): + raise BundlerError( + f"workflow '{component.id}' has invalid requirements " + f"for pinned version {component.version}." + ) + return + + from ..workflows.step.catalog._versions import validate_checksums + + validate_checksums(selected, component.id, required=True) + step_url = require_url( + selected.get("step_yml_url") or selected.get("url"), "step.yml URL" + ) + init_url = selected.get("init_url") + if init_url is None: + if not step_url.endswith("step.yml"): + raise BundlerError( + f"step '{component.id}' has no __init__.py URL " + f"for pinned version {component.version}." + ) + else: + require_url(init_url, "__init__.py URL") + for url in selected.get("extra_files", {}).values(): + require_url(url, "extra file URL") + + +def _catalog_has_release(component: ComponentRef, catalog) -> bool: + from .component_catalog import select_catalog_release, winning_catalog_entry + + current = winning_catalog_entry(catalog, component) + if current is None or not current.get("_install_allowed", True): + return False + if component.source and current.get("_catalog_name") != component.source: + return False + if component.version is None: + return True + if component.kind in ("extensions", "presets") and not current.get("version"): + # These legacy catalogs cannot attest a version; the primitive + # installer likewise accepts their unversioned current entry. + return True + selected = select_catalog_release(component, current) + if ( + selected is None + or selected.get("_catalog_name") != current.get("_catalog_name") + or not selected.get("_install_allowed", True) + or not _matches_pin(component, selected.get("version")) + ): + return False + if component.kind in ("extensions", "presets", "workflows", "steps"): + _validate_pinned_install_metadata(component, selected) + return True + + +def _resolved_in_catalog(root: Path, component: ComponentRef) -> bool | str | None: + """Return the lookup result, a validation error, or None if unreachable.""" + from ..extensions import ExtensionError + from ..presets import PresetError + from ..workflows.catalog import StepCatalogError, WorkflowCatalogError + from . import BundlerError + from .component_catalog import CatalogUnavailable + kind = component.kind try: if kind == "presets": from ..presets import PresetCatalog - return PresetCatalog(root).get_pack_info(component.id) is not None + catalog = PresetCatalog(root) + return _catalog_has_release(component, catalog) if kind == "extensions": from ..extensions import ExtensionCatalog - return ExtensionCatalog(root).get_extension_info(component.id) is not None + catalog = ExtensionCatalog(root) + return _catalog_has_release(component, catalog) if kind == "workflows": from ..workflows.catalog import WorkflowCatalog - return WorkflowCatalog(root).get_workflow_info(component.id) is not None + catalog = WorkflowCatalog(root) + return _catalog_has_release(component, catalog) if kind == "steps": from ..workflows.catalog import StepCatalog - return StepCatalog(root).get_step_info(component.id) is not None - except Exception: # noqa: BLE001 - catalog may be unreachable/misconfigured + catalog = StepCatalog(root) + return _catalog_has_release(component, catalog) + except (ConnectionError, TimeoutError): + return None + except CatalogUnavailable: return None + except (BundlerError, ExtensionError, PresetError, WorkflowCatalogError, StepCatalogError) as exc: + return f"Catalog lookup failed: {exc}" return None @@ -103,15 +226,41 @@ def check(component: ComponentRef) -> str | None: if _resolved_locally(project_root, component): return None + if component.kind in ("presets", "extensions") and component.source is None: + from .._assets import _locate_bundled_extension, _locate_bundled_preset + from . import BundlerError + from .primitives import _assert_pinned_version, _bundled_manifest_version + + kind = component.kind[:-1] + locate = ( + _locate_bundled_preset + if component.kind == "presets" + else _locate_bundled_extension + ) + bundled = locate(component.id) + if bundled is not None: + try: + _assert_pinned_version( + kind.capitalize(), + component.id, + component.version, + _bundled_manifest_version(bundled / f"{kind}.yml", kind), + ) + except BundlerError as exc: + return str(exc) + if allow_network: in_catalog = _resolved_in_catalog(project_root, component) if in_catalog is True: return None if in_catalog is False: return ( - f"{component.kind[:-1]} '{component.id}' is not bundled, " - "installed, or present in any active catalog." + f"{component.kind[:-1]} '{component.id}' at " + f"{component.version or 'current'} is not available " + "locally or from the selected install-allowed catalog." ) + if isinstance(in_catalog, str): + return in_catalog warnings.append( f"Could not verify {component.kind[:-1]} '{component.id}' " "(catalog unreachable); reference left unchecked." diff --git a/src/specify_cli/workflows/_commands.py b/src/specify_cli/workflows/_commands.py index e3dfb0bb2d..44ccf9a432 100644 --- a/src/specify_cli/workflows/_commands.py +++ b/src/specify_cli/workflows/_commands.py @@ -1036,6 +1036,7 @@ def _install_workflow_from_catalog( expected_version: str | None = None, expected_installed_version: str | None = None, requested_version: str | None = None, + selected_info: dict | None = None, ) -> None: """Download, validate, and register a catalog workflow. @@ -1045,6 +1046,8 @@ def _install_workflow_from_catalog( version does not match the catalog version that triggered the install. ``expected_installed_version``, when given by ``workflow update``, aborts if another process changes the installed source or version before commit. + ``selected_info`` is a bundle-selected catalog record, used without a + second catalog lookup while retaining the same download and commit checks. """ from .catalog import WorkflowCatalog, WorkflowCatalogError from .engine import WorkflowDefinition @@ -1061,16 +1064,19 @@ def versions_match(actual: object, expected: str) -> bool: safe_wf_id = _escape_markup(workflow_id) - catalog = WorkflowCatalog(project_root) - try: - info = ( - catalog.get_workflow_info(workflow_id, requested_version) - if requested_version is not None - else catalog.get_workflow_info(workflow_id) - ) - except WorkflowCatalogError as exc: - console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") - raise typer.Exit(1) + if selected_info is not None: + info = selected_info + else: + catalog = WorkflowCatalog(project_root) + try: + info = ( + catalog.get_workflow_info(workflow_id, requested_version) + if requested_version is not None + else catalog.get_workflow_info(workflow_id) + ) + except WorkflowCatalogError as exc: + console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") + raise typer.Exit(1) if not info: if requested_version is not None: diff --git a/src/specify_cli/workflows/command_add.py b/src/specify_cli/workflows/command_add.py index 0ea615817b..4d1cfb6567 100644 --- a/src/specify_cli/workflows/command_add.py +++ b/src/specify_cli/workflows/command_add.py @@ -30,6 +30,51 @@ def _workflow_package_has_companions(package_dir: cli.Path) -> bool: return any(path.name != "workflow.yml" for path in package_dir.iterdir()) +def _prepare_workflow_add( + project_root: cli.Path, source: str, *, dev: bool, from_url: str | None, + version: str | None, selected_catalog: bool = False, +) -> cli.Path: + from . import load_custom_steps + + if version is not None and ( + dev or from_url is not None or ( + not selected_catalog and ( + source.startswith(("http://", "https://")) + or cli.Path(source).exists() + ) + ) + ): + cli.console.print( + "[red]Error:[/red] --version requires a workflow ID from a catalog." + ) + raise cli.typer.Exit(1) + if version is not None or selected_catalog: + cli._validate_workflow_id_or_exit(source) + load_custom_steps(project_root) + cli._open_workflow_registry(project_root) + workflows_dir = project_root / ".specify" / "workflows" + if from_url is not None and not dev: + cli._validate_workflow_id_or_exit(source) + cli._reject_unsafe_dir(project_root / ".specify", ".specify") + cli._reject_unsafe_dir(workflows_dir, ".specify/workflows") + return workflows_dir + + +def _install_preselected_workflow( + workflow_id: str, *, version: str | None, selected_info: dict, +) -> None: + """Install a bundle-selected release with the normal workflow preflights.""" + project_root = cli._require_specify_project() + workflows_dir = _prepare_workflow_add( + project_root, workflow_id, dev=False, from_url=None, + version=version, selected_catalog=True, + ) + cli._install_workflow_from_catalog( + project_root, workflows_dir, workflow_id, + requested_version=version, selected_info=selected_info, + ) + + @cli.workflow_app.command("add") def workflow_add( source: str = cli.typer.Argument(..., help="Workflow ID, URL, or local path"), @@ -44,32 +89,12 @@ def workflow_add( ] = None, ): """Install a workflow from catalog, URL, or local path.""" - from . import load_custom_steps from .engine import WorkflowDefinition project_root = cli._require_specify_project() - if version is not None and ( - dev or from_url is not None or source.startswith(("http://", "https://")) - or cli.Path(source).exists() - ): - cli.console.print( - "[red]Error:[/red] --version requires a workflow ID from a catalog." - ) - raise cli.typer.Exit(1) - if version is not None: - cli._validate_workflow_id_or_exit(source) - load_custom_steps(project_root) - cli._open_workflow_registry(project_root) - workflows_dir = project_root / ".specify" / "workflows" - # With --from, source names the expected workflow ID: validate it up - # front so a URL/path/typo fails without a network fetch. - if from_url is not None and not dev: - cli._validate_workflow_id_or_exit(source) - # Reject a symlinked .specify / .specify/workflows before any write so an - # install can't escape the project root (covers the local, URL, and - # catalog branches below — all write beneath workflows_dir). - cli._reject_unsafe_dir(project_root / ".specify", ".specify") - cli._reject_unsafe_dir(workflows_dir, ".specify/workflows") + workflows_dir = _prepare_workflow_add( + project_root, source, dev=dev, from_url=from_url, version=version, + ) def _validate_and_install_local( yaml_path: cli.Path, source_label: str, expected_id: str | None = None diff --git a/src/specify_cli/workflows/step/command_add.py b/src/specify_cli/workflows/step/command_add.py index 4a0c2f55db..517fd6126c 100644 --- a/src/specify_cli/workflows/step/command_add.py +++ b/src/specify_cli/workflows/step/command_add.py @@ -259,7 +259,8 @@ def _install_from_url( def _install_from_catalog( - project_root: cli.Path, step_id: str, *, force: bool, version: str | None = None + project_root: cli.Path, step_id: str, *, force: bool, version: str | None = None, + selected_info: dict | None = None, ) -> None: """Install a step package from the step catalog. @@ -272,15 +273,18 @@ def _install_from_catalog( from .catalog import StepCatalog, StepCatalogError from .catalog._versions import validate_checksums - catalog = StepCatalog(project_root) - try: - info = ( - catalog.get_step_info(step_id, version=version) - if version is not None - else catalog.get_step_info(step_id) - ) - except StepCatalogError as exc: - raise installer.StepInstallError(str(exc)) from exc + if selected_info is not None: + info = selected_info + else: + catalog = StepCatalog(project_root) + try: + info = ( + catalog.get_step_info(step_id, version=version) + if version is not None + else catalog.get_step_info(step_id) + ) + except StepCatalogError as exc: + raise installer.StepInstallError(str(exc)) from exc if not info: raise installer.StepInstallError( @@ -538,6 +542,18 @@ def _fetch_checked(url: str, name: str) -> bytes: _print_installed(step_id, entry) +def _install_preselected_step( + step_id: str, *, version: str | None, selected_info: dict, +) -> None: + """Install a bundle-selected release with the normal step ID preflight.""" + project_root = cli._require_specify_project() + step_helpers._validate_step_id_or_exit(step_id) + _install_from_catalog( + project_root, step_id, force=False, version=version, + selected_info=selected_info, + ) + + @step_app.command("add") def workflow_step_add( step_id: str = cli.typer.Argument(..., help="Step type ID"), diff --git a/tests/specify_cli/bundles/helpers.py b/tests/specify_cli/bundles/helpers.py index e536b7e554..0d793ea5a7 100644 --- a/tests/specify_cli/bundles/helpers.py +++ b/tests/specify_cli/bundles/helpers.py @@ -132,6 +132,9 @@ def is_installed(self, project_root: Path, component: ComponentRef) -> bool: def installed_version(self, project_root: Path, component: ComponentRef) -> str | None: return self.versions.get(self._key(component)) + def validate_source(self, project_root: Path, component: ComponentRef) -> None: + pass + def install(self, project_root: Path, component: ComponentRef) -> None: from specify_cli.bundler import BundlerError @@ -139,11 +142,16 @@ def install(self, project_root: Path, component: ComponentRef) -> None: if self._fail_on is not None and component.id == self._fail_on: raise BundlerError(f"Simulated failure installing {component.id}") self.installed.add(self._key(component)) + if component.version is not None: + self.versions[self._key(component)] = component.version def remove(self, project_root: Path, component: ComponentRef) -> None: self.remove_calls.append(self._key(component)) self.installed.discard(self._key(component)) + self.versions.pop(self._key(component), None) def refresh(self, project_root: Path, component: ComponentRef) -> None: self.refresh_calls.append(self._key(component)) self.installed.add(self._key(component)) + if component.version is not None: + self.versions[self._key(component)] = component.version diff --git a/tests/specify_cli/bundles/test_command_info.py b/tests/specify_cli/bundles/test_command_info.py index 673e004ea0..954f8d019f 100644 --- a/tests/specify_cli/bundles/test_command_info.py +++ b/tests/specify_cli/bundles/test_command_info.py @@ -9,7 +9,9 @@ from typer.testing import CliRunner from specify_cli import app +from specify_cli.bundles.manifest import ComponentRef from specify_cli.bundles.packager import build_bundle # noqa: F401 +from specify_cli.bundles.records import InstalledBundleRecord, save_records from tests.conftest import strip_ansi # noqa: F401 from tests.specify_cli.bundles._command_helpers import ( MARKUP_BUNDLE_ID, @@ -101,7 +103,28 @@ def test_info_expands_full_component_set(project: Path, monkeypatch): assert preset["strategy"] == "append" assert payload["trust"] == "verified" + save_records( + project, + [ + InstalledBundleRecord.create( + bundle_id="other", + version="1.0.0", + components=[], + required_components=[ + ComponentRef(kind="presets", id="preset-a", version="2.0.0") + ], + ) + ], + ) + overlap = "preset 'preset-a' is already required by bundle 'other'." + json_with_overlap = runner.invoke( + app, ["bundle", "info", "demo-bundle", "--json", "--offline"] + ) + assert json_with_overlap.exit_code == 0, json_with_overlap.output + assert json.loads(json_with_overlap.output)["overlaps"] == [overlap] + text = runner.invoke(app, ["bundle", "info", "demo-bundle", "--offline"]) + assert overlap in text.output assert "preset-a v2.0.0" in text.output assert "Trust" in text.output diff --git a/tests/specify_cli/bundles/test_command_install.py b/tests/specify_cli/bundles/test_command_install.py index 4f1cc538e1..a87d3615c9 100644 --- a/tests/specify_cli/bundles/test_command_install.py +++ b/tests/specify_cli/bundles/test_command_install.py @@ -437,8 +437,11 @@ def download_extension_info(self, info): monkeypatch.setattr( ExtensionCatalog, - "get_extension_info", - lambda self, cid: {"id": cid, "version": version, "_install_allowed": True}, + "_fetch_single_catalog", + lambda self, entry, force_refresh=False: { + "schema_version": "1.0", + "extensions": {"catalog-ext": {"version": version}}, + }, ) monkeypatch.setattr( ExtensionCatalog, "download_extension_info", download_extension_info diff --git a/tests/specify_cli/bundles/test_command_list.py b/tests/specify_cli/bundles/test_command_list.py index 8e2fe90a9f..f97f0fda65 100644 --- a/tests/specify_cli/bundles/test_command_list.py +++ b/tests/specify_cli/bundles/test_command_list.py @@ -6,6 +6,7 @@ from unittest.mock import patch # noqa: F401 import yaml # noqa: F401 +import pytest from typer.testing import CliRunner from specify_cli import app @@ -67,6 +68,40 @@ def test_list_escapes_markup_in_records(project: Path): assert "2026-01-01T00:00:00Z[/dim]" in output +@pytest.mark.parametrize( + ("contributed", "required", "expected_count"), + [ + ([], [{"kind": "extensions", "id": "first"}, {"kind": "steps", "id": "second"}], 2), + ([{"kind": "extensions", "id": "first"}], None, 1), + ], +) +def test_list_counts_required_components( + project: Path, contributed, required, expected_count +): + record = { + "bundle_id": "demo", + "version": "1.0.0", + "installed_at": "2026-01-01T00:00:00Z", + "contributed_components": contributed, + } + if required is not None: + record["required_components"] = required + (project / ".specify" / "bundle-records.json").write_text( + json.dumps({"schema_version": "1.0", "bundles": [record]}), + encoding="utf-8", + ) + + result = runner.invoke(app, ["bundle", "list"]) + assert result.exit_code == 0, result.output + assert f"({expected_count} components," in strip_ansi(result.output) + + json_result = runner.invoke(app, ["bundle", "list", "--json"]) + assert json_result.exit_code == 0, json_result.output + parsed = json.loads(json_result.stdout)[0] + assert len(parsed["required_components"]) == expected_count + assert len(parsed["contributed_components"]) == len(contributed) + + def test_override_redirects_bundle_commands(tmp_path, monkeypatch): web = _make_project(tmp_path, "web") elsewhere = tmp_path / "elsewhere" diff --git a/tests/specify_cli/bundles/test_command_validate.py b/tests/specify_cli/bundles/test_command_validate.py index 28da55bdd8..cf4d015243 100644 --- a/tests/specify_cli/bundles/test_command_validate.py +++ b/tests/specify_cli/bundles/test_command_validate.py @@ -5,14 +5,15 @@ from pathlib import Path from unittest.mock import patch # noqa: F401 -import yaml # noqa: F401 import pytest +import yaml from typer.testing import CliRunner from specify_cli import app from specify_cli.bundles.packager import build_bundle # noqa: F401 -from tests.conftest import strip_ansi # noqa: F401 +from tests.conftest import strip_ansi from tests.specify_cli.bundles.helpers import ( + bundled_extension_version, valid_manifest_dict, ) @@ -96,10 +97,85 @@ def test_validate_rejects_broken_reference(project: Path): assert "preset-a" in result.output or "ext-a" in result.output +def test_validate_warns_instead_of_rejecting_reference_during_partial_outage( + project: Path, monkeypatch, +): + from urllib.error import URLError + + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + ) + + data = valid_manifest_dict( + provides={"steps": [{"id": "requested", "version": "1.0.0"}]} + ) + (project / "bundle.yml").write_text(yaml.safe_dump(data), encoding="utf-8") + sources = [ + StepCatalogEntry("https://example.com/high.json", "high", 1, True), + StepCatalogEntry("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(StepCatalog, "get_active_catalogs", lambda self: sources) + + def fetch(self, entry, force_refresh=False): + if entry.name == "low": + raise URLError("catalog timed out") + return {"steps": {"other-step": {"version": "1.0.0"}}} + + monkeypatch.setattr(StepCatalog, "_fetch_single_catalog", fetch) + + result = runner.invoke(app, ["bundle", "validate"]) + + assert result.exit_code == 0, result.output + assert "unreachable" in result.output + assert "not available" not in result.output + + def test_validate_accepts_bundled_reference(project: Path): data = valid_manifest_dict() - data["provides"] = {"extensions": [{"id": "agent-context", "version": "1.0.0"}]} + data["provides"] = {"extensions": [{ + "id": "agent-context", "version": bundled_extension_version("agent-context") + }]} (project / "bundle.yml").write_text(yaml.safe_dump(data), encoding="utf-8") result = runner.invoke(app, ["bundle", "validate"]) assert result.exit_code == 0, result.output assert "valid" in result.output + + +@pytest.mark.parametrize("kind,id,version", [ + ("presets", "lean", "1.0.0"), + ("extensions", "agent-context", bundled_extension_version("agent-context")), +]) +@pytest.mark.parametrize("offline", [False, True]) +def test_validate_rejects_mismatched_bundled_component( + project: Path, monkeypatch, kind: str, id: str, version: str, offline: bool, +): + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + + data = valid_manifest_dict( + provides={kind: [{"id": id, "version": "9.9.9"}]} + ) + (project / "bundle.yml").write_text(yaml.safe_dump(data), encoding="utf-8") + monkeypatch.setattr( + PresetCatalog, "get_pack_info", + lambda self, _id, version=None: { + "version": version or "9.9.9", + "_catalog_name": "trusted", + "_install_allowed": True, + }, + ) + monkeypatch.setattr( + ExtensionCatalog, "get_extension_info", + lambda self, _id, version=None: { + "version": version or "9.9.9", + "_catalog_name": "trusted", + "_install_allowed": True, + }, + ) + + command = ["bundle", "validate", *(["--offline"] if offline else [])] + result = runner.invoke(app, command) + + assert result.exit_code == 1, result.output + assert f"resolved version is {version}" in result.output diff --git a/tests/specify_cli/bundles/test_conflict.py b/tests/specify_cli/bundles/test_conflict.py index ca92f0e825..5572f2b6e3 100644 --- a/tests/specify_cli/bundles/test_conflict.py +++ b/tests/specify_cli/bundles/test_conflict.py @@ -1,9 +1,10 @@ """Unit tests for conflict detection (T034): integration clash and overlap precedence.""" from __future__ import annotations -from specify_cli.bundles.manifest import BundleManifest, ComponentRef -from specify_cli.bundles.records import InstalledBundleRecord +from specify_cli.bundles._commands import _bundle_overlaps from specify_cli.bundles.conflict import detect_conflicts +from specify_cli.bundles.manifest import BundleManifest, ComponentRef +from specify_cli.bundles.records import InstalledBundleRecord, save_records from tests.specify_cli.bundles.helpers import valid_manifest_dict @@ -36,11 +37,33 @@ def test_overlap_with_other_bundle_is_reported(): other = InstalledBundleRecord.create( bundle_id="other", version="1.0.0", - components=[ComponentRef(kind="presets", id="preset-a")], + components=[ComponentRef(kind="presets", id="preset-a", version="2.0.0")], ) report = detect_conflicts(manifest, active_integration="copilot", installed=[other]) - assert any("preset-a" in o and "other" in o for o in report.overlaps) + assert report.overlaps == [ + "preset 'preset-a' is already required by bundle 'other'." + ] + assert report.has_blocking_conflict is False + + +def test_required_only_overlap_does_not_attribute_component_ownership(tmp_path): + manifest = _manifest() + other = InstalledBundleRecord.create( + bundle_id="other", + version="1.0.0", + components=[], + required_components=[ + ComponentRef(kind="presets", id="preset-a", version="2.0.0") + ], + ) + report = detect_conflicts(manifest, active_integration="copilot", installed=[other]) + + assert report.overlaps == [ + "preset 'preset-a' is already required by bundle 'other'." + ] assert report.has_blocking_conflict is False + save_records(tmp_path, [other]) + assert _bundle_overlaps(tmp_path, manifest, offline=True) == report.overlaps def test_same_bundle_reinstall_is_not_overlap(): @@ -52,3 +75,36 @@ def test_same_bundle_reinstall_is_not_overlap(): ) report = detect_conflicts(manifest, active_integration="copilot", installed=[same]) assert report.overlaps == [] + + +def test_incompatible_pins_from_multiple_bundles_are_blocking(tmp_path): + manifest = _manifest() + records = [ + InstalledBundleRecord.create( + bundle_id=name, version="1.0.0", + components=[ComponentRef(kind="presets", id="preset-a", version=version)], + ) + for name, version in (("compatible", "v2.0.0"), ("incompatible", "3.0.0")) + ] + report = detect_conflicts(manifest, "copilot", records) + + assert report.has_blocking_conflict + assert len(report.version_clashes) == 1 + assert "incompatible" in report.version_clashes[0] + assert "3.0.0" in report.version_clashes[0] + assert len(report.overlaps) == 1 + + save_records(tmp_path, records) + assert report.version_clashes[0] in _bundle_overlaps(tmp_path, manifest, offline=True) + + +def test_unknown_existing_pin_cannot_satisfy_an_exact_pin(): + manifest = _manifest() + other = InstalledBundleRecord.create( + bundle_id="other", version="1.0.0", + components=[ComponentRef(kind="presets", id="preset-a")], + ) + report = detect_conflicts(manifest, "copilot", [other]) + + assert report.has_blocking_conflict + assert "" in report.version_clashes[0] diff --git a/tests/specify_cli/bundles/test_installer.py b/tests/specify_cli/bundles/test_installer.py index 57bf044568..6dafa36869 100644 --- a/tests/specify_cli/bundles/test_installer.py +++ b/tests/specify_cli/bundles/test_installer.py @@ -10,11 +10,20 @@ import pytest from specify_cli.bundler import BundlerError -from specify_cli.bundles.manifest import BundleManifest -from specify_cli.bundles.records import load_records, records_path from specify_cli.bundles.installer import install_bundle, remove_bundle +from specify_cli.bundles.manifest import BundleManifest, ComponentRef +from specify_cli.bundles.records import ( + InstalledBundleRecord, + load_records, + records_path, + save_records, +) from specify_cli.bundles.resolver import resolve_install_plan -from tests.specify_cli.bundles.helpers import FakeInstaller, make_project, valid_manifest_dict +from tests.specify_cli.bundles.helpers import ( + FakeInstaller, + make_project, + valid_manifest_dict, +) def _plan(manifest): @@ -51,6 +60,362 @@ def test_install_is_idempotent(tmp_path: Path): assert len(load_records(tmp_path)) == 1 +def test_second_bundle_cannot_claim_different_pin_before_mutation(tmp_path: Path): + make_project(tmp_path) + installer = FakeInstaller() + first = _bundle("first", ["ext-a"], version="1.0.0") + install_bundle(tmp_path, _plan(first), installer, manifest=first) + other = _bundle("other", ["ext-a", "ext-b"], version="2.0.0") + + with pytest.raises(BundlerError, match="Only one version"): + install_bundle(tmp_path, _plan(other), installer, manifest=other) + + assert [r.bundle_id for r in load_records(tmp_path)] == ["first"] + assert installer.install_calls == [("extensions", "ext-a")] + + +def test_independent_requirement_blocks_conflicting_bundle_after_removal( + tmp_path: Path, +): + make_project(tmp_path) + installer = FakeInstaller() + key = ("extensions", "ext-a") + installer.installed.add(key) + installer.versions[key] = "1.0.0" + first = _bundle("first", ["ext-a"], version="1.0.0") + install_bundle(tmp_path, _plan(first), installer, manifest=first) + + record = load_records(tmp_path)[0] + assert record.contributed_components == () + assert record.required_components == tuple(first.components) + + installer.installed.remove(key) + installer.versions.pop(key) + second = _bundle("second", ["ext-a"], version="2.0.0") + with pytest.raises(BundlerError, match="bundle 'first' already requires version 1.0.0"): + install_bundle(tmp_path, _plan(second), installer, manifest=second) + + assert installer.install_calls == [] + assert [r.bundle_id for r in load_records(tmp_path)] == ["first"] + + +def test_removal_preserves_component_required_but_not_owned_by_other_bundle( + tmp_path: Path, +): + make_project(tmp_path) + installer = FakeInstaller() + key = ("extensions", "ext-a") + installer.installed.add(key) + installer.versions[key] = "1.0.0" + first = _bundle("first", ["ext-a"]) + install_bundle(tmp_path, _plan(first), installer, manifest=first) + + installer.installed.remove(key) + installer.versions.pop(key) + second = _bundle("second", ["ext-a"]) + install_bundle(tmp_path, _plan(second), installer, manifest=second) + + assert load_records(tmp_path)[1].contributed_components == tuple(second.components) + result = remove_bundle(tmp_path, "second", installer) + assert key in {(c.kind, c.id) for c in result.skipped} + assert key in installer.installed + assert installer.remove_calls == [] + + remaining = load_records(tmp_path) + assert remaining[0].bundle_id == "first" + assert remaining[0].contributed_components == tuple(first.components) + remove_bundle(tmp_path, "first", installer) + assert key not in installer.installed + assert installer.remove_calls == [key] + + +def test_update_transfers_dropped_contribution_to_requiring_bundle(tmp_path: Path): + make_project(tmp_path) + installer = FakeInstaller() + first = _bundle("first", ["ext-a"]) + install_bundle(tmp_path, _plan(first), installer, manifest=first) + save_records( + tmp_path, + [ + *load_records(tmp_path), + InstalledBundleRecord.create( + "other", "1.0.0", [], required_components=first.components + ), + ], + ) + reduced = _bundle("first", []) + result = install_bundle( + tmp_path, _plan(reduced), installer, manifest=reduced, refresh=True + ) + + assert result.uninstalled == [] + assert ("extensions", "ext-a") in installer.installed + remaining = {record.bundle_id: record for record in load_records(tmp_path)} + assert remaining["first"].contributed_components == () + assert remaining["other"].contributed_components == tuple(first.components) + + remove_bundle(tmp_path, "other", installer) + assert installer.remove_calls == [("extensions", "ext-a")] + + +@pytest.mark.parametrize("actual", [None, "2.0.0"]) +def test_unpinned_install_cannot_bypass_unowned_bundle_pin( + tmp_path: Path, actual: str | None, +): + make_project(tmp_path) + installer = FakeInstaller() + key = ("steps", "shared") + installer.installed.add(key) + installer.versions[key] = "1.0.0" + pinned = _step_bundle("pinned", "1.0.0") + install_bundle(tmp_path, _plan(pinned), installer, manifest=pinned) + assert load_records(tmp_path)[0].contributed_components == () + + if actual is None: + installer.installed.remove(key) + installer.versions.pop(key) + else: + installer.versions[key] = actual + unpinned = _step_bundle("unpinned") + with pytest.raises(BundlerError, match="unpinned.*shared.*requires"): + install_bundle(tmp_path, _plan(unpinned), installer, manifest=unpinned) + + assert installer.install_calls == [] + assert [r.bundle_id for r in load_records(tmp_path)] == ["pinned"] + + +def test_unpinned_install_can_share_matching_unowned_requirement(tmp_path: Path): + make_project(tmp_path) + installer = FakeInstaller() + key = ("steps", "shared") + installer.installed.add(key) + installer.versions[key] = "1.0.0" + pinned = _step_bundle("pinned", "1.0.0") + install_bundle(tmp_path, _plan(pinned), installer, manifest=pinned) + + unpinned = _step_bundle("unpinned") + result = install_bundle(tmp_path, _plan(unpinned), installer, manifest=unpinned) + + assert result.skipped == unpinned.components + assert installer.install_calls == [] + assert load_records(tmp_path)[1].contributed_components == () + refreshed = install_bundle( + tmp_path, _plan(unpinned), installer, manifest=unpinned, refresh=True + ) + assert refreshed.skipped == unpinned.components + assert installer.refresh_calls == [] + + +def test_unpinned_refresh_cannot_change_unowned_bundle_pin(tmp_path: Path): + make_project(tmp_path) + installer = FakeInstaller() + key = ("steps", "shared") + installer.installed.add(key) + installer.versions[key] = "1.0.0" + pinned = _step_bundle("pinned", "1.0.0") + install_bundle(tmp_path, _plan(pinned), installer, manifest=pinned) + installer.installed.remove(key) + installer.versions.pop(key) + + owned = _step_bundle("owned", "1.0.0") + install_bundle(tmp_path, _plan(owned), installer, manifest=owned) + original_record = records_path(tmp_path).read_bytes() + unpinned = _step_bundle("owned") + with pytest.raises(BundlerError, match="unpinned.*shared.*requires"): + install_bundle( + tmp_path, _plan(unpinned), installer, manifest=unpinned, refresh=True + ) + + assert installer.refresh_calls == [] + assert records_path(tmp_path).read_bytes() == original_record + + +def test_owned_component_drift_is_rejected_without_refresh(tmp_path: Path): + make_project(tmp_path) + installer = FakeInstaller() + manifest = _bundle("first", ["ext-a"], version="1.0.0") + install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest) + installer.versions[("extensions", "ext-a")] = "0.9.0" + + with pytest.raises(BundlerError, match="0.9.0"): + install_bundle(tmp_path, _plan(manifest), installer, manifest=manifest) + assert installer.refresh_calls == [] + + result = install_bundle( + tmp_path, _plan(manifest), installer, manifest=manifest, refresh=True + ) + assert result.refreshed == manifest.components + + +def test_shared_component_drift_cannot_be_refreshed(tmp_path: Path): + make_project(tmp_path) + installer = FakeInstaller() + first = _bundle("first", ["ext-a"], version="1.0.0") + second = _bundle("second", ["ext-a"], version="1.0.0") + install_bundle(tmp_path, _plan(first), installer, manifest=first) + install_bundle(tmp_path, _plan(second), installer, manifest=second) + installer.versions[("extensions", "ext-a")] = "0.9.0" + + with pytest.raises(BundlerError, match="0.9.0"): + install_bundle(tmp_path, _plan(first), installer, manifest=first, refresh=True) + assert installer.refresh_calls == [] + + +@pytest.mark.parametrize( + ("change", "field"), + [ + ({"priority": 20}, "priority"), + ({"strategy": "replace"}, "strategy"), + ({"source": "trusted"}, "source"), + ], +) +@pytest.mark.parametrize("required_only", [False, True]) +def test_shared_refresh_preserves_other_bundles_install_requirements( + tmp_path: Path, change: dict, field: str, required_only: bool, +): + make_project(tmp_path) + installer = FakeInstaller() + first = _preset_bundle("first") + other = _preset_bundle("other") + install_bundle(tmp_path, _plan(first), installer, manifest=first) + if required_only: + save_records( + tmp_path, + [ + *load_records(tmp_path), + InstalledBundleRecord.create( + "other", "1.0.0", [], required_components=other.components + ), + ], + ) + else: + install_bundle(tmp_path, _plan(other), installer, manifest=other) + before = records_path(tmp_path).read_bytes() + + changed = _preset_bundle("first", **change) + with pytest.raises(BundlerError, match=rf"shared preset.*{field}"): + install_bundle( + tmp_path, _plan(changed), installer, manifest=changed, refresh=True + ) + + assert installer.refresh_calls == [] + assert records_path(tmp_path).read_bytes() == before + + +def test_shared_install_rejects_conflicting_preset_priority(tmp_path: Path): + make_project(tmp_path) + installer = FakeInstaller() + first = _preset_bundle("first") + install_bundle(tmp_path, _plan(first), installer, manifest=first) + before = records_path(tmp_path).read_bytes() + second = _preset_bundle("second", priority=20) + second.extensions.append( + ComponentRef(kind="extensions", id="ext-new", version="1.0.0") + ) + + with pytest.raises(BundlerError, match="shared preset.*priority"): + install_bundle(tmp_path, _plan(second), installer, manifest=second) + + assert installer.install_calls == [("presets", "preset-a")] + assert ("extensions", "ext-new") not in installer.installed + assert installer.refresh_calls == [] + assert records_path(tmp_path).read_bytes() == before + + +def test_shared_refresh_allows_matching_install_requirements(tmp_path: Path): + make_project(tmp_path) + installer = FakeInstaller() + first = _preset_bundle("first") + second = _preset_bundle("second") + install_bundle(tmp_path, _plan(first), installer, manifest=first) + install_bundle(tmp_path, _plan(second), installer, manifest=second) + + result = install_bundle( + tmp_path, _plan(second), installer, manifest=second, refresh=True + ) + + assert result.refreshed == second.components + assert installer.refresh_calls == [("presets", "preset-a")] + assert len(load_records(tmp_path)) == 2 + + +def test_unpinned_shared_step_cannot_refresh_another_bundles_pin(tmp_path: Path): + make_project(tmp_path) + installer = FakeInstaller() + pinned_data = valid_manifest_dict() + pinned_data["bundle"]["id"] = "pinned" + pinned_data["provides"] = {"steps": [{"id": "shared", "version": "1.0.0"}]} + pinned = BundleManifest.from_dict(pinned_data) + unpinned_data = valid_manifest_dict() + unpinned_data["bundle"]["id"] = "unpinned" + unpinned_data["provides"] = {"steps": [{"id": "shared"}]} + unpinned = BundleManifest.from_dict(unpinned_data) + install_bundle(tmp_path, _plan(pinned), installer, manifest=pinned) + install_bundle(tmp_path, _plan(unpinned), installer, manifest=unpinned) + + with pytest.raises(BundlerError, match="unpinned shared"): + install_bundle( + tmp_path, _plan(unpinned), installer, manifest=unpinned, refresh=True + ) + assert installer.refresh_calls == [] + + +@pytest.mark.parametrize("winning,accepted", [("expected", True), ("other", False)]) +def test_source_is_checked_even_when_component_is_already_installed( + tmp_path: Path, monkeypatch, winning: str, accepted: bool, +): + from specify_cli.bundles.adapters import DefaultPrimitiveInstaller + from specify_cli.extensions import ExtensionCatalog, ExtensionRegistry + + make_project(tmp_path) + ExtensionRegistry(tmp_path / ".specify" / "extensions").add( + "ext-a", {"version": "1.0.0"} + ) + fetches = [] + + def fetch(self, source, force_refresh=False): + fetches.append(source.name) + return { + "schema_version": "1.0", + "extensions": { + "ext-a": { + "version": "1.0.0", + "download_url": "https://example.com/release.zip", + } + }, + } + + from specify_cli.extensions import CatalogEntry + + monkeypatch.setattr( + ExtensionCatalog, "get_active_catalogs", + lambda self: [ + CatalogEntry("https://example.com/catalog.json", winning, 1, True) + ], + ) + monkeypatch.setattr(ExtensionCatalog, "_fetch_single_catalog", fetch) + data = valid_manifest_dict() + data["provides"] = { + "extensions": [ + {"id": "ext-a", "version": "1.0.0", "source": "expected"} + ] + } + manifest = BundleManifest.from_dict(data) + + if accepted: + result = install_bundle( + tmp_path, _plan(manifest), DefaultPrimitiveInstaller(), manifest=manifest + ) + assert result.skipped == manifest.components + assert fetches == [winning] + else: + with pytest.raises(BundlerError, match="expected"): + install_bundle( + tmp_path, _plan(manifest), DefaultPrimitiveInstaller(), manifest=manifest + ) + assert not records_path(tmp_path).exists() + + def test_install_rejects_version_change_without_refresh(tmp_path: Path): """A normal install must not advance a record past stale components. @@ -643,6 +1008,32 @@ def _bundle(manifest_id, ext_ids, *, version="1.0.0"): return BundleManifest.from_dict(data) +def _step_bundle(bundle_id: str, version: str | None = None) -> BundleManifest: + data = valid_manifest_dict() + data["bundle"]["id"] = bundle_id + step = {"id": "shared"} + if version is not None: + step["version"] = version + data["provides"] = {"steps": [step]} + return BundleManifest.from_dict(data) + + +def _preset_bundle( + bundle_id: str, *, priority: int = 10, strategy: str = "append", + source: str | None = None, +) -> BundleManifest: + data = valid_manifest_dict() + data["bundle"]["id"] = bundle_id + preset = { + "id": "preset-a", "version": "2.0.0", + "priority": priority, "strategy": strategy, + } + if source is not None: + preset["source"] = source + data["provides"] = {"presets": [preset]} + return BundleManifest.from_dict(data) + + def test_update_uninstalls_components_dropped_by_new_version(tmp_path: Path): """`bundle update` must uninstall components the new version no longer ships, instead of orphaning them (installed on disk, tracked by nothing).""" diff --git a/tests/specify_cli/bundles/test_primitives.py b/tests/specify_cli/bundles/test_primitives.py index 9cf6d3adb4..4c62012aac 100644 --- a/tests/specify_cli/bundles/test_primitives.py +++ b/tests/specify_cli/bundles/test_primitives.py @@ -12,6 +12,7 @@ import pytest from specify_cli.bundler import BundlerError +from specify_cli.bundles import component_catalog from specify_cli.bundles.adapters import DefaultPrimitiveInstaller from specify_cli.bundles.manifest import ComponentRef from specify_cli.bundles.primitives import ( @@ -31,6 +32,47 @@ def _component(kind: str, cid: str = "x") -> ComponentRef: return ComponentRef(kind=kind, id=cid) +def _patch_winning_entry(monkeypatch, entry: dict | None) -> None: + monkeypatch.setattr( + component_catalog, "winning_catalog_entry", + lambda _catalog, _component: entry, + ) + + +def _patch_catalog_sources(monkeypatch, catalog_type, kind, sources, fetches=None): + """Keep the bundle resolver real while replacing only network catalog reads.""" + configured = [ + SimpleNamespace(name=name, url=f"https://example.com/{name}.json", + install_allowed=True) + for name, _entries in sources + ] + payloads = { + source.url: {kind: entries} + for source, (_name, entries) in zip(configured, sources) + } + monkeypatch.setattr(catalog_type, "get_active_catalogs", lambda self: configured) + + def fetch(_self, source): + if fetches is not None: + fetches.append(source.name) + return payloads[source.url] + + monkeypatch.setattr(catalog_type, "_fetch_single_catalog", fetch) + + +def _workflow_or_step_entry(kind: str, cid: str) -> dict: + old = {"url": "https://example.com/old/step.yml" if kind == "steps" + else "https://example.com/old/workflow.yml"} + if kind == "steps": + old["sha256"] = {"step.yml": "a" * 64, "__init__.py": "b" * 64} + else: + old["sha256"] = "a" * 64 + return { + "id": cid, "version": "2.0.0", "url": "https://example.com/latest", + "releases": {"1.0.0": old}, + } + + def test_primitive_manager_routes_each_kind(tmp_path: Path): assert isinstance(primitive_manager("presets", tmp_path), _PresetKindManager) assert isinstance(primitive_manager("extensions", tmp_path), _ExtensionKindManager) @@ -68,14 +110,18 @@ def test_offline_step_refuses_without_network(tmp_path: Path): def test_step_manager_delegates_catalog_install_from_bundle_root(tmp_path, monkeypatch): - import specify_cli + from specify_cli.workflows.step import command_add calls: list[tuple[str, Path]] = [] - def _add(step_id: str) -> None: + def _add(project_root, step_id, **options) -> None: calls.append((step_id, Path.cwd())) - monkeypatch.setattr(specify_cli, "workflow_step_add", _add) + (tmp_path / ".specify").mkdir() + _patch_winning_entry(monkeypatch, { + "id": "catalog-step", "_catalog_name": "trusted", "_install_allowed": True, + }) + monkeypatch.setattr(command_add, "_install_from_catalog", _add) manager = _StepKindManager(tmp_path, allow_network=True) manager.install(_component("steps", "catalog-step")) @@ -83,6 +129,276 @@ def _add(step_id: str) -> None: assert calls == [("catalog-step", tmp_path)] +def test_step_manager_rejects_unsafe_id_before_catalog_lookup(tmp_path, monkeypatch): + monkeypatch.setattr( + component_catalog, "winning_catalog_entry", + lambda *args: pytest.fail("invalid ID reached catalog lookup"), + ) + + with pytest.raises(BundlerError, match="step"): + primitive_manager("steps", tmp_path).install( + _component("steps", "../outside"), + ) + + +@pytest.mark.parametrize( + "kind,catalog_module,catalog_class", + [ + ("workflows", "specify_cli.workflows.catalog", "WorkflowCatalog"), + ("steps", "specify_cli.workflows.catalog", "StepCatalog"), + ], +) +def test_catalog_workflow_and_step_pins_delegate_exact_release( + tmp_path, monkeypatch, kind, catalog_module, catalog_class, +): + import importlib + + import specify_cli._assets as assets + from specify_cli.workflows import _commands as workflow_cli + from specify_cli.workflows.step import command_add as step_add + + (tmp_path / ".specify").mkdir() + if kind == "workflows": + monkeypatch.setattr(assets, "_locate_bundled_workflow", lambda _id: None) + catalog = getattr(importlib.import_module(catalog_module), catalog_class) + fetches = [] + _patch_catalog_sources( + monkeypatch, catalog, kind, + [("trusted", {"catalog-id": _workflow_or_step_entry(kind, "catalog-id")})], + fetches, + ) + calls = [] + installer = ( + (workflow_cli, "_install_workflow_from_catalog") + if kind == "workflows" + else (step_add, "_install_from_catalog") + ) + monkeypatch.setattr( + *installer, + lambda project_root, destination, cid=None, **options: calls.append( + (cid or destination, options, Path.cwd()) + ), + ) + component = ComponentRef(kind=kind, id="catalog-id", version="1.0.0", source="trusted") + + primitive_manager(kind, tmp_path).install(component) + + assert fetches == ["trusted"] + assert calls[0][0] == component.id + assert calls[0][1][ + "requested_version" if kind == "workflows" else "version" + ] == "1.0.0" + assert calls[0][1]["selected_info"]["version"] == "1.0.0" + assert calls[0][1]["selected_info"]["_catalog_name"] == "trusted" + assert calls[0][2] == tmp_path + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +def test_catalog_component_installs_preselected_release_without_second_lookup( + tmp_path, monkeypatch, kind, +): + import specify_cli + import specify_cli._assets as assets + from specify_cli.workflows import _commands as workflow_cli + from specify_cli.workflows.catalog import StepCatalog, WorkflowCatalog + from specify_cli.workflows.step import command_add as step_add + + catalog = WorkflowCatalog if kind == "workflows" else StepCatalog + current = _workflow_or_step_entry(kind, "catalog-id") + fetches = [] + _patch_catalog_sources( + monkeypatch, catalog, kind, [("trusted", {"catalog-id": current})], fetches, + ) + (tmp_path / ".specify").mkdir() + monkeypatch.setattr( + catalog, "get_workflow_info" if kind == "workflows" else "get_step_info", + lambda *args, **kwargs: pytest.fail("installer re-resolved the catalog"), + ) + + calls = [] + if kind == "workflows": + monkeypatch.setattr(assets, "_locate_bundled_workflow", lambda _id: None) + monkeypatch.setattr( + workflow_cli, "_install_workflow_from_catalog", + lambda *args, **kwargs: calls.append(kwargs), + ) + monkeypatch.setattr( + specify_cli, "workflow_add", + lambda *args, **kwargs: pytest.fail("workflow_add re-resolved the catalog"), + ) + else: + monkeypatch.setattr( + step_add, "_install_from_catalog", + lambda *args, **kwargs: calls.append(kwargs), + ) + monkeypatch.setattr( + specify_cli, "workflow_step_add", + lambda *args, **kwargs: pytest.fail("workflow_step_add re-resolved the catalog"), + ) + + primitive_manager(kind, tmp_path).install(ComponentRef( + kind=kind, id="catalog-id", version="1.0.0", source="trusted" + )) + + assert fetches == ["trusted"] + assert len(calls) == 1 + assert calls[0]["selected_info"] == { + "id": "catalog-id", "version": "1.0.0", + **current["releases"]["1.0.0"], + "_catalog_name": "trusted", "_install_allowed": True, + } + + +@pytest.mark.parametrize("kind", ["extensions", "presets", "workflows", "steps"]) +def test_explicit_source_cannot_bypass_winning_catalog(tmp_path, monkeypatch, kind): + import specify_cli._assets as assets + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + from specify_cli.workflows.catalog import StepCatalog, WorkflowCatalog + + catalogs = { + "extensions": (ExtensionCatalog, "_locate_bundled_extension"), + "presets": (PresetCatalog, "_locate_bundled_preset"), + "workflows": (WorkflowCatalog, "_locate_bundled_workflow"), + "steps": (StepCatalog, None), + } + catalog, asset = catalogs[kind] + if asset: + monkeypatch.setattr(assets, asset, lambda _id: None) + fetches = [] + _patch_catalog_sources( + monkeypatch, catalog, kind, [ + ("winning", {"catalog-id": {"version": "2.0.0"}}), + ("lower", {"catalog-id": {"version": "1.0.0"}}), + ], fetches, + ) + component = ComponentRef(kind=kind, id="catalog-id", version="1.0.0", source="lower") + + with pytest.raises(BundlerError, match="winning"): + primitive_manager(kind, tmp_path).install(component) + assert fetches == ["winning"] + + +@pytest.mark.parametrize("kind", ["extensions", "presets", "workflows", "steps"]) +def test_primitive_install_rejects_ambiguous_catalog_source(tmp_path, monkeypatch, kind): + from specify_cli.extensions import CatalogEntry, ExtensionCatalog + from specify_cli.presets import PresetCatalog, PresetCatalogEntry + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog_type, entry_type = { + "extensions": (ExtensionCatalog, CatalogEntry), + "presets": (PresetCatalog, PresetCatalogEntry), + "workflows": (WorkflowCatalog, WorkflowCatalogEntry), + "steps": (StepCatalog, StepCatalogEntry), + }[kind] + sources = [ + entry_type("https://example.com/expected.json", "trusted", 1, True), + entry_type("https://example.com/other.json", "trusted", 2, True), + ] + monkeypatch.setattr(catalog_type, "get_active_catalogs", lambda self: sources) + monkeypatch.setattr( + catalog_type, + "_fetch_single_catalog", + lambda *_args, **_kwargs: pytest.fail( + "catalog fetch must not start with an ambiguous source" + ), + ) + + with pytest.raises(BundlerError, match="ambiguous catalog source"): + primitive_manager(kind, tmp_path).install( + ComponentRef(kind=kind, id="catalog-id", source="trusted") + ) + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +def test_missing_exact_release_never_delegates_install(tmp_path, monkeypatch, kind): + import specify_cli + import specify_cli._assets as assets + from specify_cli.workflows import command_add as workflow_add + from specify_cli.workflows.step import command_add as step_add + + if kind == "workflows": + monkeypatch.setattr(assets, "_locate_bundled_workflow", lambda _id: None) + command = ( + "workflow_add" + if kind == "workflows" + else "workflow_step_add" + ) + _patch_winning_entry(monkeypatch, { + "id": "catalog-id", "version": "2.0.0", "_catalog_name": "winning", + "_install_allowed": True, + }) + calls = [] + monkeypatch.setattr(specify_cli, command, lambda *a, **k: calls.append((a, k))) + monkeypatch.setattr( + workflow_add if kind == "workflows" else step_add, + "_install_preselected_workflow" if kind == "workflows" else "_install_preselected_step", + lambda *a, **k: calls.append((a, k)), + ) + + with pytest.raises(BundlerError, match="no catalog release"): + primitive_manager(kind, tmp_path).install( + ComponentRef(kind=kind, id="catalog-id", version="1.0.0") + ) + assert calls == [] + + +def test_bundled_workflow_with_older_pin_uses_catalog_release(tmp_path, monkeypatch): + import specify_cli._assets as assets + from specify_cli.workflows import _commands as workflow_cli + + (tmp_path / ".specify").mkdir() + bundled = _write_manifest(tmp_path / "bundled", "workflow", "2.0.0") + monkeypatch.setattr(assets, "_locate_bundled_workflow", lambda _id: bundled) + _patch_winning_entry(monkeypatch, { + **_workflow_or_step_entry("workflows", "x"), + "_catalog_name": "winning", "_install_allowed": True, + }) + calls = [] + monkeypatch.setattr( + workflow_cli, "_install_workflow_from_catalog", + lambda project_root, workflows_dir, source, **options: + calls.append((source, options)), + ) + + primitive_manager("workflows", tmp_path).install( + ComponentRef(kind="workflows", id="x", version="1.0.0") + ) + + assert calls == [("x", { + "requested_version": "1.0.0", + "selected_info": { + "id": "x", "version": "1.0.0", "_catalog_name": "winning", + "_install_allowed": True, + **_workflow_or_step_entry("workflows", "x")["releases"]["1.0.0"], + }, + })] + + +def test_source_on_bundled_extension_requires_catalog(tmp_path, monkeypatch): + import specify_cli._assets as assets + + monkeypatch.setattr( + assets, "_locate_bundled_extension", + lambda _id: _write_manifest(tmp_path / "bundled", "extension", "1.0.0"), + ) + _patch_winning_entry(monkeypatch, { + "version": "1.0.0", "_catalog_name": "other", + "_install_allowed": True, + }) + with pytest.raises(BundlerError, match="other"): + primitive_manager("extensions", tmp_path).install( + ComponentRef( + kind="extensions", id="x", version="1.0.0", source="expected" + ) + ) + + def test_default_installer_threads_allow_network(tmp_path: Path): installer = DefaultPrimitiveInstaller(allow_network=False) with pytest.raises(BundlerError, match="network access is disabled"): @@ -166,14 +482,13 @@ def test_assert_pinned_version_mismatch_raises(): def test_workflow_version_mismatch_refuses(tmp_path: Path, monkeypatch): - from specify_cli.workflows.catalog import WorkflowCatalog - - monkeypatch.setattr( - WorkflowCatalog, "get_workflow_info", lambda self, wid: {"version": "9.9.9"} - ) + _patch_winning_entry(monkeypatch, { + "id": "wf-a", "version": "9.9.9", "_catalog_name": "trusted", + "_install_allowed": True, + }) manager = primitive_manager("workflows", tmp_path, allow_network=True) component = ComponentRef(kind="workflows", id="wf-a", version="0.3.0") - with pytest.raises(BundlerError, match="pinned to version 0.3.0"): + with pytest.raises(BundlerError, match="no catalog release for pinned version 0.3.0"): manager.install(component) @@ -211,15 +526,11 @@ def install_from_zip(self, *args, **kwargs): calls.append(kwargs) monkeypatch.setattr(assets, "_locate_bundled_preset", lambda _id: None) - monkeypatch.setattr( - PresetCatalog, - "get_pack_info", - lambda _self, _id: { - "version": "1.0.0", - "_install_allowed": True, - "_catalog_name": "bundle-preset-catalog", - }, - ) + _patch_winning_entry(monkeypatch, { + "version": "1.0.0", + "_install_allowed": True, + "_catalog_name": "bundle-preset-catalog", + }) monkeypatch.setattr( PresetCatalog, "download_pack_info", lambda _self, _info: archive ) @@ -258,15 +569,11 @@ def scaffold_config(self, extension_id): scaffolded.append(extension_id) monkeypatch.setattr(assets, "_locate_bundled_extension", lambda _id: None) - monkeypatch.setattr( - ExtensionCatalog, - "get_extension_info", - lambda _self, _id: { - "version": "1.0.0", - "_install_allowed": True, - "_catalog_name": "bundle-extension-catalog", - }, - ) + _patch_winning_entry(monkeypatch, { + "version": "1.0.0", + "_install_allowed": True, + "_catalog_name": "bundle-extension-catalog", + }) monkeypatch.setattr( ExtensionCatalog, "download_extension_info", lambda _self, _info: archive ) @@ -407,11 +714,7 @@ def test_catalog_extension_install_scaffolds_config(tmp_path: Path, monkeypatch) # No bundled asset located: forces the catalog/ZIP branch (install_from_zip). monkeypatch.setattr(assets, "_locate_bundled_extension", lambda cid: None) - monkeypatch.setattr( - ExtensionCatalog, - "get_extension_info", - lambda self, eid: {"id": eid, "_install_allowed": True}, - ) + _patch_winning_entry(monkeypatch, {"id": "my-ext", "_install_allowed": True}) monkeypatch.setattr( ExtensionCatalog, "download_extension_info", lambda self, info: zip_path ) @@ -584,8 +887,9 @@ def _plan(manifest): ) +@pytest.mark.parametrize("failure_stage", ["installer", "catalog"]) def test_step_refresh_restores_registry_entry_when_reinstall_fails( - tmp_path: Path, monkeypatch + tmp_path: Path, monkeypatch, failure_stage ): """A failed step refresh must leave the registry entry restored. @@ -602,8 +906,10 @@ def test_step_refresh_restores_registry_entry_when_reinstall_fails( """ import json - import specify_cli - from specify_cli.workflows.catalog import StepRegistry + from specify_cli.bundles import primitives + from specify_cli.workflows.catalog import StepCatalogError, StepRegistry + from specify_cli.workflows.step import command_add + from specify_cli.workflows.step.installer import StepInstallError steps_dir = tmp_path / ".specify" / "workflows" / "steps" (steps_dir / "my-step").mkdir(parents=True) @@ -638,9 +944,18 @@ def test_step_refresh_restores_registry_entry_when_reinstall_fails( # Removal succeeds (real code path); only the re-install fails, which is # what a catalog 404 / size-limit / type_key mismatch produces. def _boom(step_id, *args, **kwargs): - raise BundlerError(f"Failed to install step '{step_id}'.") + raise StepInstallError(f"Failed to install step '{step_id}'.") - monkeypatch.setattr(specify_cli, "workflow_step_add", _boom) + if failure_stage == "catalog": + def _catalog_failure(*args): + raise StepCatalogError("catalog became unreachable") + + monkeypatch.setattr(primitives, "_selected_catalog_info", _catalog_failure) + else: + monkeypatch.setattr(primitives, "_selected_catalog_info", lambda *args: { + "_catalog_name": "trusted", + }) + monkeypatch.setattr(command_add, "_install_preselected_step", _boom) manager = primitive_manager("steps", tmp_path, allow_network=True) with pytest.raises(BundlerError): @@ -683,8 +998,10 @@ def _patch_extension_catalog(monkeypatch, entry: dict, archive: Path, downloads: from specify_cli.extensions import ExtensionCatalog monkeypatch.setattr(assets, "_locate_bundled_extension", lambda _id: None) + _patch_winning_entry(monkeypatch, entry) monkeypatch.setattr( - ExtensionCatalog, "get_extension_info", lambda _self, _id: entry + ExtensionCatalog, "get_extension_info", + lambda *args, **kwargs: pytest.fail("extension catalog was re-resolved"), ) def _download(_self, info): @@ -699,7 +1016,11 @@ def _patch_preset_catalog(monkeypatch, entry: dict, archive: Path, downloads: li from specify_cli.presets import PresetCatalog monkeypatch.setattr(assets, "_locate_bundled_preset", lambda _id: None) - monkeypatch.setattr(PresetCatalog, "get_pack_info", lambda _self, _id: entry) + _patch_winning_entry(monkeypatch, entry) + monkeypatch.setattr( + PresetCatalog, "get_pack_info", + lambda *args, **kwargs: pytest.fail("preset catalog was re-resolved"), + ) def _download(_self, info): downloads.append(info) diff --git a/tests/specify_cli/bundles/test_records.py b/tests/specify_cli/bundles/test_records.py index 458b0d01f0..f8caa2c5e2 100644 --- a/tests/specify_cli/bundles/test_records.py +++ b/tests/specify_cli/bundles/test_records.py @@ -38,6 +38,90 @@ def test_save_and_load_roundtrip(tmp_path: Path): ("presets", "p1"), ("steps", "s1"), } + assert loaded[0].required_components == loaded[0].contributed_components + + +def test_roundtrip_retains_unowned_required_pin(tmp_path: Path): + (tmp_path / ".specify").mkdir() + required = ComponentRef( + kind="extensions", id="independent", version="1.0.0", source="trusted" + ) + record = InstalledBundleRecord.create( + bundle_id="a", version="1.0.0", components=[], required_components=[required] + ) + + save_records(tmp_path, [record]) + restored = load_records(tmp_path)[0] + + assert restored.required_components == (required,) + assert restored.contributed_components == () + serialized = json.loads(records_path(tmp_path).read_text()) + assert serialized["bundles"][0]["required_components"] == [{ + "kind": "extensions", + "id": "independent", + "version": "1.0.0", + "source": "trusted", + }] + + +def test_legacy_record_uses_contributions_as_known_requirements(): + record = InstalledBundleRecord.create( + bundle_id="legacy", + version="1.0.0", + components=[ComponentRef(kind="extensions", id="ext-a", version="1.0.0")], + ) + data = record.to_dict() + data.pop("required_components", None) + + restored = InstalledBundleRecord.from_dict(data) + + assert restored.required_components == record.contributed_components + + +@pytest.mark.parametrize("bad", [None, 0, False, "", {}]) +def test_from_dict_rejects_invalid_required_components(bad): + data = { + "bundle_id": "a", + "version": "1.0.0", + "contributed_components": [], + "required_components": bad, + } + with pytest.raises(BundlerError, match="'required_components' must be a list"): + InstalledBundleRecord.from_dict(data) + + +@pytest.mark.parametrize( + ("component", "error"), + [ + ({"kind": "bogus", "id": "ext-a"}, "kind' must be one of"), + ({"kind": "extensions", "id": ""}, "required component is missing its 'id'"), + ], +) +def test_from_dict_rejects_corrupt_required_component(component, error): + data = { + "bundle_id": "a", + "version": "1.0.0", + "contributed_components": [], + "required_components": [component], + } + with pytest.raises(BundlerError, match=error): + InstalledBundleRecord.from_dict(data) + + +@pytest.mark.parametrize( + "required", [[], [{"kind": "extensions", "id": "ext-a", "version": "2.0.0"}]] +) +def test_from_dict_rejects_requirements_missing_contributed_pin(required): + data = { + "bundle_id": "a", + "version": "1.0.0", + "contributed_components": [ + {"kind": "extensions", "id": "ext-a", "version": "1.0.0"} + ], + "required_components": required, + } + with pytest.raises(BundlerError, match="must include all"): + InstalledBundleRecord.from_dict(data) def test_load_missing_file_returns_empty(tmp_path: Path): diff --git a/tests/specify_cli/bundles/test_references.py b/tests/specify_cli/bundles/test_references.py index a020d64a9d..7bef9206b7 100644 --- a/tests/specify_cli/bundles/test_references.py +++ b/tests/specify_cli/bundles/test_references.py @@ -6,21 +6,59 @@ from __future__ import annotations from pathlib import Path +from types import SimpleNamespace +from urllib.error import URLError + +import pytest +import yaml from specify_cli.bundles.manifest import ComponentRef from specify_cli.bundles.references import make_reference_checker -from tests.specify_cli.bundles.helpers import make_project +from tests.specify_cli.bundles.helpers import bundled_extension_version, make_project + + +def _ref(kind: str, id_: str, version: str | None = "1.0.0") -> ComponentRef: + return ComponentRef(kind=kind, id=id_, version=version) + + +def _mock_catalog(monkeypatch, catalog_type, kind, component_id, record, *, name="trusted"): + source = SimpleNamespace( + name=name, url="https://example.com/catalog.json", install_allowed=True + ) + monkeypatch.setattr(catalog_type, "get_active_catalogs", lambda self: [source]) + monkeypatch.setattr( + catalog_type, + "_fetch_single_catalog", + lambda self, entry, force_refresh=False: { + "schema_version": "1.0", kind: {component_id: record} + }, + ) -def _ref(kind: str, id_: str) -> ComponentRef: - return ComponentRef(kind=kind, id=id_, version="1.0.0") +def _installable_current(kind: str) -> dict: + if kind in ("extensions", "presets"): + return { + "version": "1.0.0", + "download_url": "https://example.com/release.zip", + "sha256": "a" * 64, + } + return { + "version": "1.0.0", + "url": f"https://example.com/{'workflow.yml' if kind == 'workflows' else 'step.yml'}", + "sha256": ( + "a" * 64 if kind == "workflows" + else {"step.yml": "a" * 64, "__init__.py": "b" * 64} + ), + } def test_bundled_extension_resolves(tmp_path: Path): root = make_project(tmp_path) warnings: list[str] = [] check = make_reference_checker(root, allow_network=True, warnings=warnings) - assert check(_ref("extensions", "agent-context")) is None + assert check( + _ref("extensions", "agent-context", bundled_extension_version("agent-context")) + ) is None assert warnings == [] @@ -42,7 +80,7 @@ def test_builtin_step_type_resolves(tmp_path: Path): for step_id in ("shell", "gate", "command", "if", "slot"): assert step_id in BUILTIN_STEP_TYPES, step_id - assert check(_ref("steps", step_id)) is None, step_id + assert check(_ref("steps", step_id, None)) is None, step_id assert warnings == [] @@ -85,8 +123,11 @@ def execute(self, config, context): # pragma: no cover - never run STEP_REGISTRY.pop("community-only-step", None) -def test_unknown_step_type_still_errors_online(tmp_path: Path): +def test_unknown_step_type_still_errors_online(tmp_path: Path, monkeypatch): """The guard must not make every step id resolve.""" + from specify_cli.workflows.catalog import StepCatalog + + _mock_catalog(monkeypatch, StepCatalog, "steps", "other-step", {"version": "1.0.0"}) root = make_project(tmp_path) warnings: list[str] = [] check = make_reference_checker(root, allow_network=True, warnings=warnings) @@ -110,3 +151,1235 @@ def test_unknown_reference_warns_offline(tmp_path: Path): check = make_reference_checker(root, allow_network=False, warnings=warnings) assert check(_ref("presets", "does-not-exist")) is None assert any("does-not-exist" in w for w in warnings) + + +@pytest.mark.parametrize("allow_network", [False, True]) +def test_wrong_bundled_extension_pin_is_definitive( + tmp_path, monkeypatch, allow_network, +): + from specify_cli.extensions import ExtensionCatalog + + root = make_project(tmp_path) + _mock_catalog( + monkeypatch, ExtensionCatalog, "extensions", + "agent-context", { + "version": "999.0.0", + "download_url": "https://example.com/release.zip", + }, + ) + warnings = [] + check = make_reference_checker(root, allow_network=allow_network, warnings=warnings) + + problem = check(_ref("extensions", "agent-context", "999.0.0")) + assert problem is not None and "resolved version is" in problem + assert warnings == [] + if allow_network: + assert check(ComponentRef( + kind="extensions", id="agent-context", version="999.0.0", source="trusted" + )) is None + assert warnings == [] + assert check( + _ref("extensions", "agent-context", bundled_extension_version("agent-context")) + ) is None + + +@pytest.mark.parametrize("allow_network", [False, True]) +def test_bundled_preset_pin_mismatch_is_definitive( + tmp_path, monkeypatch, allow_network, +): + import specify_cli._assets as assets + from specify_cli.presets import PresetCatalog + + bundled = tmp_path / "preset" + bundled.mkdir() + (bundled / "preset.yml").write_text( + "preset:\n id: requested\n version: 1.0.0\n", encoding="utf-8" + ) + monkeypatch.setattr(assets, "_locate_bundled_preset", lambda _id: bundled) + _mock_catalog( + monkeypatch, PresetCatalog, "presets", "requested", + {"version": "2.0.0", "download_url": "https://example.com/release.zip"} + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=allow_network, warnings=warnings) + + problem = check(_ref("presets", "requested", "2.0.0")) + assert problem is not None and "resolved version is 1.0.0" in problem + assert warnings == [] + if allow_network: + assert check(ComponentRef( + kind="presets", id="requested", version="2.0.0", source="trusted" + )) is None + assert warnings == [] + assert check(_ref("presets", "requested", "1.0.0")) is None + + +def test_bundled_preset_mismatch_allows_matching_installed_version( + tmp_path, monkeypatch, +): + from types import SimpleNamespace + + import specify_cli._assets as assets + from specify_cli.bundles import primitives + + bundled = tmp_path / "preset" + bundled.mkdir() + (bundled / "preset.yml").write_text( + "preset:\n id: requested\n version: 1.0.0\n", encoding="utf-8" + ) + monkeypatch.setattr(assets, "_locate_bundled_preset", lambda _id: bundled) + monkeypatch.setattr( + primitives, "primitive_manager", + lambda *args, **kwargs: SimpleNamespace( + is_installed=lambda _component: True, + installed_version=lambda _component: "2.0.0", + ), + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=False, warnings=warnings) + + assert check(_ref("presets", "requested", "2.0.0")) is None + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +def test_bundle_http_protocol_error_is_unreachable_catalog( + tmp_path, monkeypatch, kind, +): + from http.client import BadStatusLine + + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + + catalog_type = ExtensionCatalog if kind == "extensions" else PresetCatalog + _mock_catalog(monkeypatch, catalog_type, kind, "requested", {}) + + def bad_status(self, source, force_refresh=False): + raise BadStatusLine("bad response") + + monkeypatch.setattr(catalog_type, "_fetch_single_catalog", bad_status) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + assert check(_ref(kind, "requested")) is None + assert len(warnings) == 1 and "unreachable" in warnings[0] + + +def test_online_validation_warns_for_extension_http_protocol_error( + tmp_path, monkeypatch, +): + from http.client import BadStatusLine + + from specify_cli.extensions import CatalogEntry, ExtensionCatalog + + monkeypatch.setattr( + ExtensionCatalog, "get_active_catalogs", + lambda self: [CatalogEntry("https://example.com/catalog.json", "trusted", 1, True)], + ) + + def bad_status(*args, **kwargs): + raise BadStatusLine("bad response") + + monkeypatch.setattr(ExtensionCatalog, "_open_url", bad_status) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(_ref("extensions", "requested")) is None + assert len(warnings) == 1 and "unreachable" in warnings[0] + + +def test_online_validation_checks_winning_exact_release_and_source(tmp_path, monkeypatch): + import specify_cli._assets as assets + from specify_cli.workflows.catalog import WorkflowCatalog + + monkeypatch.setattr(assets, "_locate_bundled_workflow", lambda _id: None) + fetches = [] + _mock_catalog( + monkeypatch, WorkflowCatalog, "workflows", "catalog-workflow", + { + "version": "2.0.0", + "releases": { + "1.0.0": { + "url": "https://example.com/old.yml", + "sha256": "a" * 64, + } + }, + }, + name="winning", + ) + original_fetch = WorkflowCatalog._fetch_single_catalog + + def fetch(self, source, force_refresh=False): + fetches.append(source.name) + return original_fetch(self, source, force_refresh) + + monkeypatch.setattr(WorkflowCatalog, "_fetch_single_catalog", fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + requested = ComponentRef( + kind="workflows", id="catalog-workflow", version="1.0.0", source="winning" + ) + + assert check(requested) is None + assert fetches == ["winning"] + assert check(ComponentRef(kind="workflows", id=requested.id, version="3.0.0")) + assert check(ComponentRef( + kind="workflows", id=requested.id, version="1.0.0", source="lower" + )) + assert warnings == [] + + +def test_online_validation_rejects_discovery_only_exact_release(tmp_path, monkeypatch): + from specify_cli.workflows.catalog import StepCatalog, StepCatalogEntry + + source = StepCatalogEntry("https://example.com/catalog.json", "winning", 1, False) + monkeypatch.setattr(StepCatalog, "get_active_catalogs", lambda self: [source]) + monkeypatch.setattr( + StepCatalog, "_fetch_single_catalog", + lambda self, entry, force_refresh=False: { + "steps": {"catalog-step": {"version": "2.0.0"}} + }, + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(_ref("steps", "catalog-step", "1.0.0")) + assert warnings == [] + + +def test_online_validation_reports_invalid_release_metadata(tmp_path, monkeypatch): + from specify_cli.workflows.catalog import StepCatalog + + _mock_catalog( + monkeypatch, StepCatalog, "steps", "invalid-release", + { + "version": "2.0.0", + "releases": { + "1.0.0": {"step_yml_url": "https://example.com/step.yml"} + }, + }, + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert "SHA-256" in check(_ref("steps", "invalid-release")) + assert warnings == [] + + +@pytest.mark.parametrize( + ("kind", "field", "bad_value", "message"), + [ + ("workflows", "url", None, "install URL"), + ("workflows", "url", 42, "install URL"), + ("workflows", "url", "http://example.com/workflow.yml", "install URL"), + ("workflows", "sha256", "not-a-digest", "SHA-256"), + ("steps", "url", None, "step.yml URL"), + ("steps", "url", 42, "step.yml URL"), + ("steps", "url", "http://example.com/step.yml", "step.yml URL"), + ("steps", "url", "https://example.com/other.yml", "__init__.py URL"), + ("steps", "init_url", "http://example.com/__init__.py", "__init__.py URL"), + ("steps", "sha256", {"step.yml": "a" * 64}, "SHA-256"), + ("steps", "extra_files", {"helper.py": "http://example.com/helper.py"}, "extra file URL"), + ], +) +def test_online_validation_rejects_malformed_pinned_current_release( + tmp_path, monkeypatch, kind, field, bad_value, message, +): + from specify_cli.workflows.catalog import StepCatalog, WorkflowCatalog + + catalog = WorkflowCatalog if kind == "workflows" else StepCatalog + record = _installable_current(kind) + record[field] = bad_value + if field == "extra_files": + record["sha256"]["helper.py"] = "c" * 64 + _mock_catalog(monkeypatch, catalog, kind, "requested", record) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + problem = check(_ref(kind, "requested")) + assert problem is not None and message in problem + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +def test_online_validation_accepts_installable_pinned_current_release( + tmp_path, monkeypatch, kind, +): + from specify_cli.workflows.catalog import StepCatalog, WorkflowCatalog + + catalog = WorkflowCatalog if kind == "workflows" else StepCatalog + record = _installable_current(kind) + _mock_catalog(monkeypatch, catalog, kind, "requested", record) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(_ref(kind, "requested")) is None + assert warnings == [] + + +def test_online_validation_warns_when_catalogs_are_unreachable(tmp_path, monkeypatch): + from specify_cli.workflows.catalog import WorkflowCatalog + + _mock_catalog(monkeypatch, WorkflowCatalog, "workflows", "unreachable", {}) + + def unavailable(self, source, force_refresh=False): + raise URLError("timed out") + + monkeypatch.setattr(WorkflowCatalog, "_fetch_single_catalog", unavailable) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(_ref("workflows", "unreachable")) is None + assert len(warnings) == 1 + assert "unreachable" in warnings[0] + + +@pytest.mark.parametrize("kind", ["extensions", "presets", "workflows", "steps"]) +@pytest.mark.parametrize( + ("unreachable", "has_match"), + [ + ("high", False), + ("low", False), + ("high", True), + ("low", True), + (None, False), + ], +) +def test_online_validation_distinguishes_partial_outage_from_missing_reference( + tmp_path, monkeypatch, kind, unreachable, has_match, +): + from specify_cli.extensions import CatalogEntry, ExtensionCatalog + from specify_cli.presets import PresetCatalog, PresetCatalogEntry + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry_type = { + "extensions": (ExtensionCatalog, CatalogEntry), + "presets": (PresetCatalog, PresetCatalogEntry), + "workflows": (WorkflowCatalog, WorkflowCatalogEntry), + "steps": (StepCatalog, StepCatalogEntry), + }[kind] + sources = [ + entry_type("https://example.com/high.json", "high", 1, True), + entry_type("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(catalog, "get_active_catalogs", lambda self: sources) + visited = [] + + def fetch(self, entry, force_refresh=False): + visited.append(entry.name) + if entry.name == unreachable: + raise URLError("catalog timed out") + contents = {"requested": _installable_current(kind)} if has_match else {} + return {kind: contents} + + monkeypatch.setattr(catalog, "_fetch_single_catalog", fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + problem = check(_ref(kind, "requested")) + if unreachable == "high": + assert problem is None + assert len(warnings) == 1 + assert "unreachable" in warnings[0] + assert visited == ["high"] + elif has_match and unreachable == "low": + assert problem is None + assert warnings == [] + assert visited == ["high"] + elif unreachable is not None: + assert problem is None + assert len(warnings) == 1 + assert "unreachable" in warnings[0] + assert visited == ["high", "low"] + elif has_match: + assert problem is None + assert warnings == [] + assert visited == ["high"] + else: + assert problem is not None and "not available" in problem + assert warnings == [] + assert visited == ["high", "low"] + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +def test_online_validation_warns_for_unreachable_component_catalog( + tmp_path, monkeypatch, kind, +): + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + + catalog = ExtensionCatalog if kind == "extensions" else PresetCatalog + _mock_catalog(monkeypatch, catalog, kind, "unreachable-component", {}) + + def unavailable(self, source, force_refresh=False): + raise URLError("timed out") + + monkeypatch.setattr(catalog, "_fetch_single_catalog", unavailable) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(_ref(kind, "unreachable-component")) is None + assert len(warnings) == 1 + assert "unreachable" in warnings[0] + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +def test_online_validation_rejects_malformed_component_release( + tmp_path, monkeypatch, kind, +): + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + + catalog = ExtensionCatalog if kind == "extensions" else PresetCatalog + _mock_catalog( + monkeypatch, catalog, kind, "invalid-component", + { + "version": "2.0.0", + "releases": { + "1.0.0": {"download_url": "https://example.com/release.zip"} + }, + }, + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert "SHA-256" in check(_ref(kind, "invalid-component")) + assert warnings == [] + + +@pytest.mark.parametrize("historical", [False, True]) +@pytest.mark.parametrize( + "url", + ["http://example.com/release.zip", "https://[::1", "https:///release.zip", 42], +) +def test_online_validation_rejects_invalid_pinned_extension_url( + tmp_path, monkeypatch, historical, url, +): + from specify_cli.extensions import ExtensionCatalog + + record = {"download_url": url, "sha256": "a" * 64} + current = {"version": "2.0.0", **record} + if historical: + current["releases"] = {"1.0.0": record} + _mock_catalog( + monkeypatch, ExtensionCatalog, "extensions", "invalid-extension", current + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + problem = check(_ref( + "extensions", "invalid-extension", "1.0.0" if historical else "2.0.0" + )) + assert problem is not None and ("URL" in problem or "download_url" in problem) + assert warnings == [] + + +@pytest.mark.parametrize("historical", [False, True]) +def test_online_validation_accepts_safe_pinned_extension_url( + tmp_path, monkeypatch, historical, +): + from specify_cli.extensions import ExtensionCatalog + + record = {"download_url": "https://example.com/release.zip", "sha256": "a" * 64} + current = {"version": "2.0.0", **record} + if historical: + current["releases"] = {"1.0.0": record} + _mock_catalog(monkeypatch, ExtensionCatalog, "extensions", "valid-extension", current) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(_ref( + "extensions", "valid-extension", "1.0.0" if historical else "2.0.0" + )) is None + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +@pytest.mark.parametrize( + "change", + [ + {"download_url": None}, + {"download_url": "http://example.com/release.zip"}, + {"sha256": "not-a-digest"}, + ], +) +def test_online_validation_rejects_malformed_current_component_metadata( + tmp_path, monkeypatch, kind, change, +): + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + + catalog = ExtensionCatalog if kind == "extensions" else PresetCatalog + _mock_catalog( + monkeypatch, catalog, kind, "malformed-current", + {**_installable_current(kind), **change}, + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + problem = check(_ref(kind, "malformed-current")) + assert problem is not None and "Catalog lookup failed" in problem + assert warnings == [] + + +@pytest.mark.parametrize("digest", [None, "a" * 64, "sha256:" + "A" * 64]) +def test_online_validation_accepts_current_preset_metadata(tmp_path, monkeypatch, digest): + from specify_cli.presets import PresetCatalog + + record = _installable_current("presets") + record["sha256"] = digest + _mock_catalog( + monkeypatch, PresetCatalog, "presets", "valid-current", record, + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(_ref("presets", "valid-current")) is None + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +def test_online_validation_accepts_unversioned_legacy_component( + tmp_path, monkeypatch, kind, +): + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + + catalog = ExtensionCatalog if kind == "extensions" else PresetCatalog + _mock_catalog(monkeypatch, catalog, kind, "legacy-component", {}) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(_ref(kind, "legacy-component", "1.0.0")) is None + assert warnings == [] + + +def test_online_validation_rejects_malformed_extension_catalog( + tmp_path, monkeypatch, +): + from specify_cli.extensions import ( + CatalogEntry, + ExtensionCatalog, + ) + + monkeypatch.setattr( + ExtensionCatalog, "get_active_catalogs", + lambda self: [CatalogEntry("https://example.com/catalog.json", "trusted", 1, True)], + ) + monkeypatch.setattr( + ExtensionCatalog, "_fetch_single_catalog", + lambda self, entry, force_refresh=False: + self._validate_catalog_payload({"extensions": []}, entry.url), + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert "Invalid catalog format" in check(_ref("extensions", "invalid")) + assert warnings == [] + + +def test_online_validation_rejects_malformed_winning_extension_catalog( + tmp_path, monkeypatch, +): + from specify_cli.extensions import CatalogEntry, ExtensionCatalog + + sources = [ + CatalogEntry("https://example.com/high.json", "high", 1, True), + CatalogEntry("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(ExtensionCatalog, "get_active_catalogs", lambda self: sources) + + def fetch(self, entry, force_refresh=False): + if entry.name == "high": + self._validate_catalog_payload({"extensions": []}, entry.url) + return { + "schema_version": "1.0", + "extensions": {"requested": {"version": "1.0.0"}}, + } + + monkeypatch.setattr(ExtensionCatalog, "_fetch_single_catalog", fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert "Invalid catalog format" in check(_ref("extensions", "requested")) + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +@pytest.mark.parametrize("payload", [b"{invalid", b"[]"]) +def test_online_validation_rejects_malformed_workflow_catalogs( + tmp_path, monkeypatch, kind, payload, +): + import io + + from specify_cli.authentication import http + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry = ( + (WorkflowCatalog, WorkflowCatalogEntry) + if kind == "workflows" + else (StepCatalog, StepCatalogEntry) + ) + url = "https://example.com/catalog.json" + monkeypatch.setattr( + catalog, "get_active_catalogs", + lambda self: [entry(url, "trusted", 1, True)], + ) + + class Response(io.BytesIO): + def geturl(self): + return url + + monkeypatch.setattr(http, "open_url", lambda *args, **kwargs: Response(payload)) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert "Catalog lookup failed" in check(_ref(kind, "requested")) + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +def test_online_validation_warns_for_unreachable_workflow_catalogs( + tmp_path, monkeypatch, kind, +): + from urllib.error import URLError + + from specify_cli.authentication import http + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry = ( + (WorkflowCatalog, WorkflowCatalogEntry) + if kind == "workflows" + else (StepCatalog, StepCatalogEntry) + ) + monkeypatch.setattr( + catalog, "get_active_catalogs", + lambda self: [ + entry("https://example.com/catalog.json", "trusted", 1, True), + ], + ) + + def unavailable(*args, **kwargs): + raise URLError("timed out") + + monkeypatch.setattr(http, "open_url", unavailable) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(_ref(kind, "requested")) is None + assert len(warnings) == 1 + assert "unreachable" in warnings[0] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +def test_online_validation_rejects_unsafe_catalog_redirect( + tmp_path, monkeypatch, kind, +): + from specify_cli.authentication import http + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry = ( + (WorkflowCatalog, WorkflowCatalogEntry) + if kind == "workflows" + else (StepCatalog, StepCatalogEntry) + ) + monkeypatch.setattr( + catalog, "get_active_catalogs", + lambda self: [ + entry("https://example.com/catalog.json", "trusted", 1, True), + ], + ) + + def unsafe_redirect(*args, **kwargs): + raise http.RedirectPolicyError("unsafe catalog redirect") + + monkeypatch.setattr(http, "open_url", unsafe_redirect) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert "unsafe catalog redirect" in check(_ref(kind, "requested")) + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +@pytest.mark.parametrize("redirect", ["validator", "policy"]) +def test_online_validation_does_not_skip_unsafe_higher_priority_catalog( + tmp_path, monkeypatch, kind, redirect, +): + import io + import json + + from specify_cli.authentication.http import RedirectPolicyError + from specify_cli.extensions import CatalogEntry, ExtensionCatalog + from specify_cli.presets import PresetCatalog, PresetCatalogEntry + + catalog, entry = ( + (ExtensionCatalog, CatalogEntry) + if kind == "extensions" + else (PresetCatalog, PresetCatalogEntry) + ) + sources = [ + entry("https://example.com/high.json", "high", 1, True), + entry("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(catalog, "get_active_catalogs", lambda self: sources) + + class Response(io.BytesIO): + def __init__(self, url, payload): + super().__init__(payload) + self.url = url + + def geturl(self): + return self.url + + def open_url(self, url, **kwargs): + if url.endswith("high.json"): + if redirect == "validator": + kwargs["redirect_validator"](url, "http://evil.test/catalog.json") + pytest.fail("unsafe redirect was accepted") + raise RedirectPolicyError("unsafe catalog redirect") + return Response( + url, + json.dumps({ + "schema_version": "1.0", + kind: { + "requested": { + "version": "1.0.0", + "download_url": "https://example.com/archive.zip", + "sha256": "a" * 64, + } + }, + }).encode(), + ) + + monkeypatch.setattr(catalog, "_open_url", open_url) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + problem = check(_ref(kind, "requested")) + assert problem is not None and ( + "HTTPS" in problem or "unsafe catalog redirect" in problem + ) + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +def test_bundle_invalid_utf8_catalog_is_validation_error( + tmp_path, monkeypatch, kind, +): + import io + + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + + class Response(io.BytesIO): + def geturl(self): + return "https://example.com/catalog.json" + + catalog_type = ExtensionCatalog if kind == "extensions" else PresetCatalog + original_fetch = catalog_type._fetch_single_catalog + _mock_catalog(monkeypatch, catalog_type, kind, "requested", {}) + monkeypatch.setattr(catalog_type, "_open_url", lambda self, url, **kw: Response(b"\xff")) + monkeypatch.setattr(catalog_type, "_fetch_single_catalog", original_fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + assert "decode" in check(_ref(kind, "requested")) + assert warnings == [] + + +def test_online_validation_does_not_skip_invalid_utf8_extension_catalog( + tmp_path, monkeypatch, +): + import io + import json + + from specify_cli.extensions import CatalogEntry, ExtensionCatalog + + sources = [ + CatalogEntry("https://example.com/high.json", "high", 1, True), + CatalogEntry("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(ExtensionCatalog, "get_active_catalogs", lambda self: sources) + + class Response(io.BytesIO): + def __init__(self, url, payload): + super().__init__(payload) + self.url = url + + def geturl(self): + return self.url + + def open_url(self, url, **kwargs): + payload = ( + b"\xff" if url.endswith("high.json") + else json.dumps({ + "schema_version": "1.0", + "extensions": {"requested": {"version": "1.0.0"}}, + }).encode() + ) + return Response(url, payload) + + monkeypatch.setattr(ExtensionCatalog, "_open_url", open_url) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert "decode" in check(_ref("extensions", "requested")) + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["extensions", "presets"]) +@pytest.mark.parametrize("status", [404, 503]) +def test_bundle_catalog_http_errors_preserve_failure_type( + tmp_path, monkeypatch, kind, status, +): + from urllib.error import HTTPError + + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + + catalog_type = ExtensionCatalog if kind == "extensions" else PresetCatalog + _mock_catalog(monkeypatch, catalog_type, kind, "requested", {}) + + def failed(self, source, force_refresh=False): + raise HTTPError(source.url, status, "catalog failure", {}, None) + + monkeypatch.setattr(catalog_type, "_fetch_single_catalog", failed) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + problem = check(_ref(kind, "requested")) + if status == 503: + assert problem is None + assert len(warnings) == 1 and "unreachable" in warnings[0] + else: + assert problem is not None and "404" in problem + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +@pytest.mark.parametrize("payload", [b"[]", b"{}", b'{"schema_version":"1.0"}']) +@pytest.mark.parametrize("cached", [False, True]) +def test_online_validation_does_not_skip_malformed_higher_priority_catalog( + tmp_path, monkeypatch, kind, payload, cached, +): + import io + import json + + from specify_cli.authentication import http + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry = ( + (WorkflowCatalog, WorkflowCatalogEntry) + if kind == "workflows" + else (StepCatalog, StepCatalogEntry) + ) + sources = [ + entry("https://example.com/high.json", "high", 1, True), + entry("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(catalog, "get_active_catalogs", lambda self: sources) + if cached: + catalog_instance = catalog(tmp_path) + cache_file, _ = catalog_instance._get_cache_paths(sources[0].url) + cache_file.parent.mkdir(parents=True, exist_ok=True) + cache_file.write_bytes(payload) + monkeypatch.setattr( + catalog, "_is_url_cache_valid", + lambda self, url: url == sources[0].url, + ) + + class Response(io.BytesIO): + def __init__(self, url, payload): + super().__init__(payload) + self.url = url + + def geturl(self): + return self.url + + def open_url(url, **kwargs): + response_payload = ( + payload if url.endswith("high.json") + else json.dumps({ + kind: {"requested": {"version": "1.0.0"}}, + }).encode() + ) + return Response(url, response_payload) + + monkeypatch.setattr(http, "open_url", open_url) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert "Catalog lookup failed" in check(ComponentRef(kind=kind, id="requested")) + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +@pytest.mark.parametrize("empty", [{}, []]) +def test_valid_empty_higher_priority_catalog_allows_lower_source( + tmp_path, monkeypatch, kind, empty, +): + import io + import json + + from specify_cli.authentication import http + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry = ( + (WorkflowCatalog, WorkflowCatalogEntry) + if kind == "workflows" + else (StepCatalog, StepCatalogEntry) + ) + sources = [ + entry("https://example.com/high.json", "high", 1, True), + entry("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(catalog, "get_active_catalogs", lambda self: sources) + + class Response(io.BytesIO): + def __init__(self, url, payload): + super().__init__(payload) + self.url = url + + def geturl(self): + return self.url + + def open_url(url, **kwargs): + entries = empty if url.endswith("high.json") else { + "requested": {"version": "1.0.0"} + } + return Response(url, json.dumps({kind: entries}).encode()) + + monkeypatch.setattr(http, "open_url", open_url) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(ComponentRef(kind=kind, id="requested")) is None + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +@pytest.mark.parametrize("source", ["network", "fresh-cache", "stale-cache"]) +def test_bundle_catalog_handles_recursion_without_fallback( + tmp_path, monkeypatch, kind, source, +): + from specify_cli.workflows.catalog import ( + StepCatalog, + WorkflowCatalog, + ) + + catalog_type = WorkflowCatalog if kind == "workflows" else StepCatalog + _mock_catalog(monkeypatch, catalog_type, kind, "requested", _installable_current(kind)) + + def fetch(self, entry, force_refresh=False): + if source == "fresh-cache": + return {kind: {"requested": _installable_current(kind)}} + if source == "stale-cache": + raise URLError("connection failed") + raise RecursionError("catalog nesting limit exceeded") + + monkeypatch.setattr(catalog_type, "_fetch_single_catalog", fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + problem = check(_ref(kind, "requested")) + if source == "network": + assert problem is not None and "nesting limit exceeded" in problem + assert warnings == [] + elif source == "stale-cache": + assert problem is None + assert len(warnings) == 1 and "unreachable" in warnings[0] + else: + assert problem is None + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +def test_bundle_catalog_handles_cache_write_recursion(tmp_path, monkeypatch, kind): + from specify_cli.workflows.catalog import StepCatalog, WorkflowCatalog + + catalog_type = WorkflowCatalog if kind == "workflows" else StepCatalog + _mock_catalog(monkeypatch, catalog_type, kind, "requested", {"version": "1.0.0"}) + + def fetch(self, entry, force_refresh=False): + raise RecursionError("cache write nesting limit exceeded") + + monkeypatch.setattr(catalog_type, "_fetch_single_catalog", fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + assert "cache write nesting limit exceeded" in check(_ref(kind, "requested")) + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["extensions", "workflows", "steps"]) +def test_targeted_lookup_stops_before_lower_priority_catalog( + tmp_path, monkeypatch, kind, +): + from specify_cli.extensions import CatalogEntry, ExtensionCatalog + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry, key = { + "extensions": (ExtensionCatalog, CatalogEntry, "extensions"), + "workflows": (WorkflowCatalog, WorkflowCatalogEntry, "workflows"), + "steps": (StepCatalog, StepCatalogEntry, "steps"), + }[kind] + sources = [ + entry("https://example.com/high.json", "high", 1, True), + entry("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(catalog, "get_active_catalogs", lambda self: sources) + + def fetch(self, source, force_refresh=False): + if source.name != "high": + pytest.fail("lower-priority catalog was fetched after finding the ID") + return { + "schema_version": "1.0", + key: {"requested": {"version": "1.0.0"}}, + } + + monkeypatch.setattr(catalog, "_fetch_single_catalog", fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + assert check(ComponentRef(kind=kind, id="requested", source="high")) is None + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["extensions", "presets", "workflows", "steps"]) +@pytest.mark.parametrize("malformed", [None, 42, "invalid", True]) +def test_bundle_rejects_malformed_higher_source_before_lower_match( + tmp_path, monkeypatch, kind, malformed, +): + from specify_cli.extensions import CatalogEntry, ExtensionCatalog + from specify_cli.presets import PresetCatalog, PresetCatalogEntry + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry_type = { + "extensions": (ExtensionCatalog, CatalogEntry), + "presets": (PresetCatalog, PresetCatalogEntry), + "workflows": (WorkflowCatalog, WorkflowCatalogEntry), + "steps": (StepCatalog, StepCatalogEntry), + }[kind] + sources = [ + entry_type("https://example.com/high.json", "high", 1, True), + entry_type("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(catalog, "get_active_catalogs", lambda self: sources) + fetched = [] + + def fetch(self, source, force_refresh=False): + fetched.append(source.name) + if source.name == "high": + return {"schema_version": "1.0", kind: malformed} + return {"schema_version": "1.0", kind: {"requested": {"version": "1.0.0"}}} + + monkeypatch.setattr(catalog, "_fetch_single_catalog", fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + problem = check(_ref(kind, "requested")) + assert problem is not None and "malformed" in problem + assert fetched == ["high"] + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +@pytest.mark.parametrize("duplicate_id", ["requested", "other"]) +def test_bundle_rejects_duplicate_list_ids_before_lower_source( + tmp_path, monkeypatch, kind, duplicate_id, +): + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry_type = ( + (WorkflowCatalog, WorkflowCatalogEntry) + if kind == "workflows" else (StepCatalog, StepCatalogEntry) + ) + sources = [ + entry_type("https://example.com/high.json", "high", 1, True), + entry_type("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(catalog, "get_active_catalogs", lambda self: sources) + visited = [] + + def fetch(self, source, force_refresh=False): + visited.append(source.name) + if source.name == "low": + pytest.fail("duplicate IDs in the higher source were ignored") + return {kind: [ + {"id": "requested", "version": "1.0.0"}, + {"id": duplicate_id, "version": "2.0.0"}, + {"id": duplicate_id, "version": "3.0.0"}, + ]} + + monkeypatch.setattr(catalog, "_fetch_single_catalog", fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + assert f"Duplicate {kind[:-1]} ID '{duplicate_id}'" in check(_ref(kind, "requested")) + assert visited == ["high"] + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +@pytest.mark.parametrize("invalid_id", [{"bad": "id"}, ["bad"], 42, True, None, ""]) +def test_bundle_rejects_invalid_list_ids( + tmp_path, monkeypatch, kind, invalid_id, +): + from specify_cli.workflows.catalog import StepCatalog, WorkflowCatalog + + catalog = WorkflowCatalog if kind == "workflows" else StepCatalog + _mock_catalog(monkeypatch, catalog, kind, "requested", {}) + monkeypatch.setattr( + catalog, "_fetch_single_catalog", + lambda self, source, force_refresh=False: { + kind: [ + {"id": "requested", "version": "1.0.0"}, + {"id": invalid_id, "version": "2.0.0"}, + ] + }, + ) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + assert f"Invalid {kind[:-1]} ID" in check(_ref(kind, "requested")) + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["workflows", "steps"]) +def test_bundle_accepts_list_id_from_first_source(tmp_path, monkeypatch, kind): + from specify_cli.workflows.catalog import ( + StepCatalog, + StepCatalogEntry, + WorkflowCatalog, + WorkflowCatalogEntry, + ) + + catalog, entry_type = ( + (WorkflowCatalog, WorkflowCatalogEntry) + if kind == "workflows" else (StepCatalog, StepCatalogEntry) + ) + sources = [ + entry_type("https://example.com/high.json", "high", 1, True), + entry_type("https://example.com/low.json", "low", 2, True), + ] + monkeypatch.setattr(catalog, "get_active_catalogs", lambda self: sources) + visited = [] + + def fetch(self, source, force_refresh=False): + visited.append(source.name) + return {kind: [{"id": "requested", "version": source.name}]} + + monkeypatch.setattr(catalog, "_fetch_single_catalog", fetch) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + assert check(ComponentRef(kind=kind, id="requested")) is None + assert visited == ["high"] + assert warnings == [] + + +@pytest.mark.parametrize("kind", ["extensions", "presets", "workflows", "steps"]) +@pytest.mark.parametrize("duplicate_name", [False, True]) +def test_explicit_source_requires_unique_catalog_name( + tmp_path, monkeypatch, kind, duplicate_name, +): + from specify_cli.bundles import BundlerError + from specify_cli.bundles.adapters import DefaultPrimitiveInstaller + from specify_cli.extensions import ExtensionCatalog + from specify_cli.presets import PresetCatalog + from specify_cli.workflows.catalog import ( + StepCatalog, + WorkflowCatalog, + ) + + catalog_type, env_key = { + "extensions": (ExtensionCatalog, "SPECKIT_CATALOG_URL"), + "presets": (PresetCatalog, "SPECKIT_PRESET_CATALOG_URL"), + "workflows": ( + WorkflowCatalog, "SPECKIT_WORKFLOW_CATALOG_URL" + ), + "steps": (StepCatalog, "SPECKIT_STEP_CATALOG_URL"), + }[kind] + monkeypatch.delenv(env_key, raising=False) + config = tmp_path / ".specify" / f"{kind[:-1]}-catalogs.yml" + config.parent.mkdir() + config.write_text( + yaml.safe_dump({"catalogs": [ + { + "url": "https://example.com/expected.json", + "name": "trusted", + "priority": 1, + "install_allowed": True, + }, + { + "url": "https://example.com/other.json", + "name": "trusted" if duplicate_name else "other", + "priority": 2, + "install_allowed": True, + }, + ]}), + encoding="utf-8", + ) + assert [entry.name for entry in catalog_type(tmp_path).get_active_catalogs()] == [ + "trusted", + "trusted" if duplicate_name else "other", + ] + monkeypatch.setattr( + catalog_type, + "_fetch_single_catalog", + lambda self, source, force_refresh=False: { + "schema_version": "1.0", + kind: {"requested": {"version": "1.0.0"}}, + }, + ) + ref = ComponentRef(kind=kind, id="requested", source="trusted") + warnings: list[str] = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + installer = DefaultPrimitiveInstaller() + + if duplicate_name: + assert "ambiguous" in check(ref).lower() + with pytest.raises(BundlerError, match="ambiguous"): + installer.validate_source(tmp_path, ref) + else: + assert check(ref) is None + installer.validate_source(tmp_path, ref) + assert warnings == [] diff --git a/tests/specify_cli/bundles/test_validator.py b/tests/specify_cli/bundles/test_validator.py index 855fb80cc1..19b6ba380e 100644 --- a/tests/specify_cli/bundles/test_validator.py +++ b/tests/specify_cli/bundles/test_validator.py @@ -3,8 +3,8 @@ import pytest -from specify_cli.bundles.manifest import BundleManifest from specify_cli.bundles import validator as validator_mod +from specify_cli.bundles.manifest import BundleManifest from specify_cli.bundles.validator import validate_manifest from tests.specify_cli.bundles.helpers import valid_manifest_dict @@ -20,6 +20,19 @@ def test_invalid_speckit_constraint_reported_as_error(): assert any("speckit_version" in e for e in report.errors) +def test_duplicate_component_id_cannot_request_two_versions(): + data = valid_manifest_dict() + data["provides"]["extensions"].append({"id": "ext-a", "version": "2.0.0"}) + + report = validate_manifest(BundleManifest.from_dict(data)) + + assert not report.ok + assert any( + "ext-a" in error and "duplicate" in error.lower() + for error in report.errors + ) + + def test_non_bundler_error_not_swallowed(monkeypatch): # A programming error inside constraint parsing must propagate, not be # masked behind an "invalid constraint" validation message. diff --git a/tests/specify_cli/workflows/step/test_command_add.py b/tests/specify_cli/workflows/step/test_command_add.py index 191398b3f5..8eda3941e4 100644 --- a/tests/specify_cli/workflows/step/test_command_add.py +++ b/tests/specify_cli/workflows/step/test_command_add.py @@ -1700,7 +1700,8 @@ def _setup(project_dir, monkeypatch, *, discovery=False, corrupt=None): elif corrupt == "url": entry["releases"]["1.0"]["init_url"] = "http://evil.example/__init__.py" monkeypatch.setattr( - StepCatalog, "_get_merged_steps", lambda self: {"deploy": entry} + StepCatalog, "_get_merged_steps", + lambda self, *, step_id=None: {"deploy": entry} ) requested = [] @@ -1756,6 +1757,48 @@ def test_install_exact_selected_files(self, project_dir, monkeypatch): project_dir / ".specify/workflows/steps/deploy/helper.py" ).read_bytes() == b"# helper\n" + def test_preselected_step_installs_without_reloading_catalog( + self, project_dir, monkeypatch, + ): + from specify_cli.workflows.step.catalog import StepCatalog, StepRegistry + from specify_cli.workflows.step.command_add import _install_preselected_step + + requested = self._setup(project_dir, monkeypatch) + selected = StepCatalog(project_dir).get_step_info("deploy", version="1.0") + monkeypatch.setattr( + StepCatalog, "get_step_info", + lambda *args, **kwargs: pytest.fail("catalog was re-resolved"), + ) + monkeypatch.chdir(project_dir) + _install_preselected_step("deploy", version="1.0", selected_info=selected) + + assert requested == [ + f"https://example.com/old/{name}" + for name in ("step.yml", "__init__.py", "helper.py") + ] + assert StepRegistry(project_dir).get("deploy")["version"] == "1.0" + + def test_preselected_step_rejects_bad_digest_without_reloading_catalog( + self, project_dir, monkeypatch, + ): + from specify_cli.workflows.step.catalog import StepCatalog, StepRegistry + from specify_cli.workflows.step.command_add import _install_preselected_step + from specify_cli.workflows.step.installer import StepInstallError + + self._setup(project_dir, monkeypatch, corrupt="checksum") + selected = StepCatalog(project_dir).get_step_info("deploy", version="1.0") + monkeypatch.setattr( + StepCatalog, "get_step_info", + lambda *args, **kwargs: pytest.fail("catalog was re-resolved"), + ) + monkeypatch.chdir(project_dir) + + with pytest.raises(StepInstallError, match="checksum mismatch"): + _install_preselected_step( + "deploy", version="1.0", selected_info=selected, + ) + assert not StepRegistry(project_dir).is_installed("deploy") + @pytest.mark.parametrize( ("corrupt", "error"), [ diff --git a/tests/specify_cli/workflows/test_catalog_versions.py b/tests/specify_cli/workflows/test_catalog_versions.py index 6a1900f7be..2eafe38594 100644 --- a/tests/specify_cli/workflows/test_catalog_versions.py +++ b/tests/specify_cli/workflows/test_catalog_versions.py @@ -8,6 +8,7 @@ from unittest.mock import patch import pytest +import typer from typer.testing import CliRunner from specify_cli import app @@ -259,6 +260,89 @@ def open_url(url, **kwargs): assert WorkflowRegistry(project_dir).get("history-wf")["version"] == "1.0.0" +def test_preselected_workflow_installs_without_reloading_catalog( + monkeypatch, project_dir, +): + from specify_cli.authentication import http + from specify_cli.workflows.command_add import _install_preselected_workflow + + catalog = _catalog(monkeypatch, project_dir, _entry()) + selected = catalog.get_workflow_info("history-wf", "1.0.0") + assert selected["url"] == OLD_URL + monkeypatch.setattr( + WorkflowCatalog, "get_workflow_info", + lambda *args, **kwargs: pytest.fail("catalog was re-resolved"), + ) + requested = [] + + def open_url(url, **kwargs): + requested.append(url) + return _Response(_archive("1.0.0", requires={"integrations": ["copilot"]}), url) + + monkeypatch.setattr(http, "open_url", open_url) + monkeypatch.chdir(project_dir) + _install_preselected_workflow( + "history-wf", version="1.0.0", selected_info=selected, + ) + + assert requested == [OLD_URL] + assert WorkflowRegistry(project_dir).get("history-wf")["version"] == "1.0.0" + + +def test_preselected_workflow_ignores_same_named_local_path( + monkeypatch, project_dir, +): + from specify_cli.authentication import http + from specify_cli.workflows.command_add import _install_preselected_workflow + + catalog = _catalog(monkeypatch, project_dir, _entry()) + selected = catalog.get_workflow_info("history-wf", "1.0.0") + shadow = project_dir / "history-wf" + shadow.write_text("not a workflow", encoding="utf-8") + monkeypatch.setattr( + WorkflowCatalog, "get_workflow_info", + lambda *args, **kwargs: pytest.fail("catalog was re-resolved"), + ) + monkeypatch.setattr( + http, "open_url", + lambda url, **kwargs: _Response( + _archive("1.0.0", requires={"integrations": ["copilot"]}), url + ), + ) + monkeypatch.chdir(project_dir) + + _install_preselected_workflow( + "history-wf", version="1.0.0", selected_info=selected, + ) + + assert WorkflowRegistry(project_dir).get("history-wf")["version"] == "1.0.0" + assert shadow.read_text(encoding="utf-8") == "not a workflow" + + +def test_preselected_workflow_rejects_bad_digest_without_reloading_catalog( + monkeypatch, project_dir, +): + from specify_cli.authentication import http + from specify_cli.workflows.command_add import _install_preselected_workflow + catalog = _catalog(monkeypatch, project_dir, _entry()) + selected = catalog.get_workflow_info("history-wf", "1.0.0") + selected["sha256"] = "0" * 64 + monkeypatch.setattr( + WorkflowCatalog, "get_workflow_info", + lambda *args, **kwargs: pytest.fail("catalog was re-resolved"), + ) + monkeypatch.setattr( + http, "open_url", lambda url, **kwargs: _Response(_archive("1.0.0"), url), + ) + monkeypatch.chdir(project_dir) + + with pytest.raises(typer.Exit): + _install_preselected_workflow( + "history-wf", version="1.0.0", selected_info=selected, + ) + assert WorkflowRegistry(project_dir).get("history-wf") is None + + def test_exact_yaml_release_verifies_digest_and_version(monkeypatch, project_dir): from specify_cli.authentication import http