From d2ecf6560de325449e47e9498a9395bb74d81dc1 Mon Sep 17 00:00:00 2001 From: Michal Bachorik Date: Tue, 17 Mar 2026 15:44:34 +0100 Subject: [PATCH] feat(extensions,presets): add priority-based resolution ordering (#1855) * feat(extensions,presets): add priority-based resolution ordering Add priority field to extension and preset registries for deterministic template resolution when multiple sources provide the same template. Extensions: - Add `list_by_priority()` method to ExtensionRegistry - Add `--priority` option to `extension add` command - Add `extension set-priority` command - Show priority in `extension list` and `extension info` - Preserve priority during `extension update` - Update RFC documentation Presets: - Add `preset set-priority` command - Show priority in `preset info` output - Use priority ordering in PresetResolver for extensions Both systems: - Lower priority number = higher precedence (default: 10) - Backwards compatible with legacy entries (missing priority defaults to 10) - Comprehensive test coverage including backwards compatibility Closes #1845 Closes #1854 Co-Authored-By: Claude Opus 4.5 * fix: address code review feedback - list_by_priority(): add secondary sort by ID for deterministic ordering, return deep copies to prevent mutation - install_from_directory/zip: validate priority >= 1 early - extension add CLI: validate --priority >= 1 before install - PresetRegistry.update(): preserve installed_at timestamp - Test assertions: use exact source string instead of substring match Co-Authored-By: Claude Opus 4.5 * fix: address additional review feedback - PresetResolver: add fallback to directory scanning when registry is empty/corrupted for robustness and backwards compatibility - PresetRegistry.update(): add guard to prevent injecting installed_at when absent in existing entry (mirrors ExtensionRegistry behavior) - RFC: update extension list example to match actual CLI output format Co-Authored-By: Claude Opus 4.5 * fix: restore defensive code and RFC descriptions lost in rebase - Restore defensive code in list_by_priority() with .get() and isinstance check - Restore detailed --from URL and --dev option descriptions in RFC Co-Authored-By: Claude Opus 4.5 * fix: add defensive code to presets list_by_priority() - Add .get() and isinstance check for corrupted/empty registry - Move copy import to module level (remove local import) - Matches defensive pattern used in extensions.py Co-Authored-By: Claude Opus 4.5 * fix: address reviewer feedback on priority resolution - Rename _normalize_priority to normalize_priority (public API) - Add comprehensive tests for normalize_priority function (9 tests) - Filter non-dict metadata entries in list_by_priority() methods - Fix extension priority resolution to merge registered and unregistered extensions into unified sorted list (unregistered get implicit priority 10) - Add tests for extension priority resolution ordering (4 tests) The key fix ensures unregistered extensions with implicit priority 10 correctly beat registered extensions with priority > 10, and vice versa. Co-Authored-By: Claude Opus 4.5 * fix: DRY refactor and strengthen test assertions - Extract _get_all_extensions_by_priority() helper in PresetResolver to eliminate duplicated extension list construction - Add priority=10 assertion to test_legacy_extension_without_priority_field - Add priority=10 assertion to test_legacy_preset_without_priority_field Co-Authored-By: Claude Opus 4.5 * fix: add isinstance(dict) checks for corrupted registry entries Add defensive checks throughout CLI commands and manager methods to handle cases where registry entries may be corrupted (non-dict values). This prevents AttributeError when calling .get() on non-dict metadata. Locations fixed: - __init__.py: preset/extension info, set-priority, enable/disable, upgrade commands - extensions.py: list_installed() - presets.py: list_installed() Co-Authored-By: Claude Opus 4.5 * fix: normalize priority display to match resolution behavior Use normalize_priority() for all priority display in CLI commands to ensure displayed values match actual resolution behavior when registry data is corrupted/hand-edited. Locations fixed: - extensions.py: list_installed() - presets.py: list_installed(), PresetResolver - __init__.py: preset info, extension info, set-priority commands Also added GraphQL query for unresolved PR comments to CLAUDE.md. Co-Authored-By: Claude Opus 4.5 * fix: repair corrupted priority values in set-priority commands Changed set-priority commands to check if the raw stored value is already a valid int equal to the requested priority before skipping. This ensures corrupted values (e.g., "high") get repaired even when setting to the default priority (10). Also removed CLAUDE.md that was accidentally added to the repo. Co-Authored-By: Claude Opus 4.5 * fix: harden registry update methods against corrupted entries - Normalize priority when restoring during extension update to prevent propagating corrupted values (e.g., "high", 0, negative) - Add isinstance(dict) checks in ExtensionRegistry.update() and PresetRegistry.update() to handle corrupted entries (string/list) that would cause TypeError on merge Co-Authored-By: Claude Opus 4.5 * fix: use safe fallback for version in list_installed() When registry entry is corrupted (non-dict), metadata becomes {} after the isinstance check. Use metadata.get("version", manifest.version) instead of metadata["version"] to avoid KeyError. Co-Authored-By: Claude Opus 4.5 --------- Co-authored-by: iamaeroplane Co-authored-by: Claude Opus 4.5 --- extensions/RFC-EXTENSION-SYSTEM.md | 47 +++- src/specify_cli/__init__.py | 168 +++++++++++- src/specify_cli/extensions.py | 77 +++++- src/specify_cli/presets.py | 160 +++++++++-- tests/test_extensions.py | 427 +++++++++++++++++++++++++++++ tests/test_presets.py | 295 +++++++++++++++++++- 6 files changed, 1113 insertions(+), 61 deletions(-) diff --git a/extensions/RFC-EXTENSION-SYSTEM.md b/extensions/RFC-EXTENSION-SYSTEM.md index a0f6034e..5d3d8e9c 100644 --- a/extensions/RFC-EXTENSION-SYSTEM.md +++ b/extensions/RFC-EXTENSION-SYSTEM.md @@ -359,12 +359,15 @@ specify extension add jira "installed_at": "2026-01-28T14:30:00Z", "source": "catalog", "manifest_hash": "sha256:abc123...", - "enabled": true + "enabled": true, + "priority": 10 } } } ``` +**Priority Field**: Extensions are ordered by `priority` (lower = higher precedence). Default is 10. Used for template resolution when multiple extensions provide the same template. + ### 3. Configuration ```bash @@ -1084,11 +1087,15 @@ List installed extensions in current project. $ specify extension list Installed Extensions: - ✓ jira (v1.0.0) - Jira Integration - Commands: 3 | Hooks: 2 | Status: Enabled + ✓ Jira Integration (v1.0.0) + jira + Create Jira issues from spec-kit artifacts + Commands: 3 | Hooks: 2 | Priority: 10 | Status: Enabled - ✓ linear (v0.9.0) - Linear Integration - Commands: 1 | Hooks: 1 | Status: Enabled + ✓ Linear Integration (v0.9.0) + linear + Create Linear issues from spec-kit artifacts + Commands: 1 | Hooks: 1 | Priority: 10 | Status: Enabled ``` **Options:** @@ -1196,10 +1203,9 @@ Next steps: **Options:** -- `--from URL`: Install from custom URL or Git repo -- `--version VERSION`: Install specific version -- `--dev PATH`: Install from local path (development mode) -- `--no-register`: Skip command registration (manual setup) +- `--from URL`: Install from a remote URL (archive). Does not accept Git repositories directly. +- `--dev`: Install from a local path in development mode (the PATH is the positional `extension` argument). +- `--priority NUMBER`: Set resolution priority (lower = higher precedence, default 10) #### `specify extension remove NAME` @@ -1280,6 +1286,29 @@ $ specify extension disable jira To re-enable: specify extension enable jira ``` +#### `specify extension set-priority NAME PRIORITY` + +Change the resolution priority of an installed extension. + +```bash +$ specify extension set-priority jira 5 + +✓ Extension 'Jira Integration' priority changed: 10 → 5 + +Lower priority = higher precedence in template resolution +``` + +**Priority Values:** + +- Lower numbers = higher precedence (checked first in resolution) +- Default priority is 10 +- Must be a positive integer (1 or higher) + +**Use Cases:** + +- Ensure a critical extension's templates take precedence +- Override default resolution order when multiple extensions provide similar templates + --- ## Compatibility & Versioning diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 8509db7e..ec1ccf97 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2000,6 +2000,11 @@ def preset_add( console.print("Run this command from a spec-kit project root") raise typer.Exit(1) + # Validate priority + if priority < 1: + console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") + raise typer.Exit(1) + manager = PresetManager(project_root) speckit_version = get_speckit_version() @@ -2177,6 +2182,7 @@ def preset_info( pack_id: str = typer.Argument(..., help="Preset ID to get info about"), ): """Show detailed information about a preset.""" + from .extensions import normalize_priority from .presets import PresetCatalog, PresetManager, PresetError project_root = Path.cwd() @@ -2210,6 +2216,10 @@ def preset_info( if license_val: console.print(f" License: {license_val}") console.print("\n [green]Status: installed[/green]") + # Get priority from registry + pack_metadata = manager.registry.get(pack_id) + priority = normalize_priority(pack_metadata.get("priority") if isinstance(pack_metadata, dict) else None) + console.print(f" [dim]Priority:[/dim] {priority}") console.print() return @@ -2241,6 +2251,58 @@ def preset_info( console.print() +@preset_app.command("set-priority") +def preset_set_priority( + pack_id: str = typer.Argument(help="Preset ID"), + priority: int = typer.Argument(help="New priority (lower = higher precedence)"), +): + """Set the resolution priority of an installed preset.""" + from .presets import PresetManager + + project_root = Path.cwd() + + # Check if we're in a spec-kit project + specify_dir = project_root / ".specify" + if not specify_dir.exists(): + console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)") + console.print("Run this command from a spec-kit project root") + raise typer.Exit(1) + + # Validate priority + if priority < 1: + console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") + raise typer.Exit(1) + + manager = PresetManager(project_root) + + # Check if preset is installed + if not manager.registry.is_installed(pack_id): + console.print(f"[red]Error:[/red] Preset '{pack_id}' is not installed") + raise typer.Exit(1) + + # Get current metadata + metadata = manager.registry.get(pack_id) + if metadata is None or not isinstance(metadata, dict): + console.print(f"[red]Error:[/red] Preset '{pack_id}' not found in registry (corrupted state)") + raise typer.Exit(1) + + from .extensions import normalize_priority + raw_priority = metadata.get("priority") + # Only skip if the stored value is already a valid int equal to requested priority + # This ensures corrupted values (e.g., "high") get repaired even when setting to default (10) + if isinstance(raw_priority, int) and raw_priority == priority: + console.print(f"[yellow]Preset '{pack_id}' already has priority {priority}[/yellow]") + raise typer.Exit(0) + + old_priority = normalize_priority(raw_priority) + + # Update priority + manager.registry.update(pack_id, {"priority": priority}) + + console.print(f"[green]✓[/green] Preset '{pack_id}' priority changed: {old_priority} → {priority}") + console.print("\n[dim]Lower priority = higher precedence in template resolution[/dim]") + + # ===== Preset Catalog Commands ===== @@ -2578,7 +2640,7 @@ def extension_list( console.print(f" [{status_color}]{status_icon}[/{status_color}] [bold]{ext['name']}[/bold] (v{ext['version']})") console.print(f" [dim]{ext['id']}[/dim]") console.print(f" {ext['description']}") - console.print(f" Commands: {ext['command_count']} | Hooks: {ext['hook_count']} | Status: {'Enabled' if ext['enabled'] else 'Disabled'}") + console.print(f" Commands: {ext['command_count']} | Hooks: {ext['hook_count']} | Priority: {ext['priority']} | Status: {'Enabled' if ext['enabled'] else 'Disabled'}") console.print() if available or all_extensions: @@ -2766,6 +2828,7 @@ def extension_add( extension: str = typer.Argument(help="Extension name or path"), dev: bool = typer.Option(False, "--dev", help="Install from local directory"), from_url: Optional[str] = typer.Option(None, "--from", help="Install from custom URL"), + priority: int = typer.Option(10, "--priority", help="Resolution priority (lower = higher precedence, default 10)"), ): """Install an extension.""" from .extensions import ExtensionManager, ExtensionCatalog, ExtensionError, ValidationError, CompatibilityError @@ -2779,6 +2842,11 @@ def extension_add( console.print("Run this command from a spec-kit project root") raise typer.Exit(1) + # Validate priority + if priority < 1: + console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") + raise typer.Exit(1) + manager = ExtensionManager(project_root) speckit_version = get_speckit_version() @@ -2795,7 +2863,7 @@ def extension_add( console.print(f"[red]Error:[/red] No extension.yml found in {source_path}") raise typer.Exit(1) - manifest = manager.install_from_directory(source_path, speckit_version) + manifest = manager.install_from_directory(source_path, speckit_version, priority=priority) elif from_url: # Install from URL (ZIP file) @@ -2828,7 +2896,7 @@ def extension_add( zip_path.write_bytes(zip_data) # Install from downloaded ZIP - manifest = manager.install_from_zip(zip_path, speckit_version) + manifest = manager.install_from_zip(zip_path, speckit_version, priority=priority) except urllib.error.URLError as e: console.print(f"[red]Error:[/red] Failed to download from {from_url}: {e}") raise typer.Exit(1) @@ -2872,7 +2940,7 @@ def extension_add( try: # Install from downloaded ZIP - manifest = manager.install_from_zip(zip_path, speckit_version) + manifest = manager.install_from_zip(zip_path, speckit_version, priority=priority) finally: # Clean up downloaded ZIP if zip_path.exists(): @@ -3048,7 +3116,7 @@ def extension_info( extension: str = typer.Argument(help="Extension ID or name"), ): """Show detailed information about an extension.""" - from .extensions import ExtensionCatalog, ExtensionManager + from .extensions import ExtensionCatalog, ExtensionManager, normalize_priority project_root = Path.cwd() @@ -3085,8 +3153,15 @@ def extension_info( # Get local manifest info ext_manifest = manager.get_extension(resolved_installed_id) metadata = manager.registry.get(resolved_installed_id) + metadata_is_dict = isinstance(metadata, dict) + if not metadata_is_dict: + console.print( + "[yellow]Warning:[/yellow] Extension metadata appears to be corrupted; " + "some information may be unavailable." + ) + version = metadata.get("version", "unknown") if metadata_is_dict else "unknown" - console.print(f"\n[bold]{resolved_installed_name}[/bold] (v{metadata.get('version', 'unknown')})") + console.print(f"\n[bold]{resolved_installed_name}[/bold] (v{version})") console.print(f"ID: {resolved_installed_id}") console.print() @@ -3114,6 +3189,8 @@ def extension_info( console.print() console.print("[green]✓ Installed[/green]") + priority = normalize_priority(metadata.get("priority") if metadata_is_dict else None) + console.print(f"[dim]Priority:[/dim] {priority}") console.print(f"\nTo remove: specify extension remove {resolved_installed_id}") return @@ -3129,6 +3206,8 @@ def extension_info( def _print_extension_info(ext_info: dict, manager): """Print formatted extension info from catalog data.""" + from .extensions import normalize_priority + # Header verified_badge = " [green]✓ Verified[/green]" if ext_info.get("verified") else "" console.print(f"\n[bold]{ext_info['name']}[/bold] (v{ext_info['version']}){verified_badge}") @@ -3207,6 +3286,9 @@ def _print_extension_info(ext_info: dict, manager): install_allowed = ext_info.get("_install_allowed", True) if is_installed: console.print("[green]✓ Installed[/green]") + metadata = manager.registry.get(ext_info['id']) + priority = normalize_priority(metadata.get("priority") if isinstance(metadata, dict) else None) + console.print(f"[dim]Priority:[/dim] {priority}") console.print(f"\nTo remove: specify extension remove {ext_info['id']}") elif install_allowed: console.print("[yellow]Not installed[/yellow]") @@ -3233,6 +3315,7 @@ def extension_update( ValidationError, CommandRegistrar, HookExecutor, + normalize_priority, ) from packaging import version as pkg_version import shutil @@ -3272,7 +3355,7 @@ def extension_update( for ext_id in extensions_to_update: # Get installed version metadata = manager.registry.get(ext_id) - if metadata is None or "version" not in metadata: + if metadata is None or not isinstance(metadata, dict) or "version" not in metadata: console.print(f"⚠ {ext_id}: Registry entry corrupted or missing (skipping)") continue try: @@ -3457,13 +3540,13 @@ def extension_update( shutil.copy2(cfg_file, new_extension_dir / cfg_file.name) # 9. Restore metadata from backup (installed_at, enabled state) - if backup_registry_entry: + if backup_registry_entry and isinstance(backup_registry_entry, dict): # Copy current registry entry to avoid mutating internal # registry state before explicit restore(). current_metadata = manager.registry.get(extension_id) - if current_metadata is None: + if current_metadata is None or not isinstance(current_metadata, dict): raise RuntimeError( - f"Registry entry for '{extension_id}' missing after install — update incomplete" + f"Registry entry for '{extension_id}' missing or corrupted after install — update incomplete" ) new_metadata = dict(current_metadata) @@ -3471,6 +3554,10 @@ def extension_update( if "installed_at" in backup_registry_entry: new_metadata["installed_at"] = backup_registry_entry["installed_at"] + # Preserve the original priority (normalized to handle corruption) + if "priority" in backup_registry_entry: + new_metadata["priority"] = normalize_priority(backup_registry_entry["priority"]) + # If extension was disabled before update, disable it again if not backup_registry_entry.get("enabled", True): new_metadata["enabled"] = False @@ -3524,7 +3611,7 @@ def extension_update( # (files that weren't in the original backup) try: new_registry_entry = manager.registry.get(extension_id) - if new_registry_entry is None: + if new_registry_entry is None or not isinstance(new_registry_entry, dict): new_registered_commands = {} else: new_registered_commands = new_registry_entry.get("registered_commands", {}) @@ -3644,10 +3731,10 @@ def extension_enable( # Update registry metadata = manager.registry.get(extension_id) - if metadata is None: + if metadata is None or not isinstance(metadata, dict): console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)") raise typer.Exit(1) - + if metadata.get("enabled", True): console.print(f"[yellow]Extension '{display_name}' is already enabled[/yellow]") raise typer.Exit(0) @@ -3692,10 +3779,10 @@ def extension_disable( # Update registry metadata = manager.registry.get(extension_id) - if metadata is None: + if metadata is None or not isinstance(metadata, dict): console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)") raise typer.Exit(1) - + if not metadata.get("enabled", True): console.print(f"[yellow]Extension '{display_name}' is already disabled[/yellow]") raise typer.Exit(0) @@ -3717,6 +3804,57 @@ def extension_disable( console.print(f"To re-enable: specify extension enable {extension_id}") +@extension_app.command("set-priority") +def extension_set_priority( + extension: str = typer.Argument(help="Extension ID or name"), + priority: int = typer.Argument(help="New priority (lower = higher precedence)"), +): + """Set the resolution priority of an installed extension.""" + from .extensions import ExtensionManager + + project_root = Path.cwd() + + # Check if we're in a spec-kit project + specify_dir = project_root / ".specify" + if not specify_dir.exists(): + console.print("[red]Error:[/red] Not a spec-kit project (no .specify/ directory)") + console.print("Run this command from a spec-kit project root") + raise typer.Exit(1) + + # Validate priority + if priority < 1: + console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") + raise typer.Exit(1) + + manager = ExtensionManager(project_root) + + # Resolve extension ID from argument (handles ambiguous names) + installed = manager.list_installed() + extension_id, display_name = _resolve_installed_extension(extension, installed, "set-priority") + + # Get current metadata + metadata = manager.registry.get(extension_id) + if metadata is None or not isinstance(metadata, dict): + console.print(f"[red]Error:[/red] Extension '{extension_id}' not found in registry (corrupted state)") + raise typer.Exit(1) + + from .extensions import normalize_priority + raw_priority = metadata.get("priority") + # Only skip if the stored value is already a valid int equal to requested priority + # This ensures corrupted values (e.g., "high") get repaired even when setting to default (10) + if isinstance(raw_priority, int) and raw_priority == priority: + console.print(f"[yellow]Extension '{display_name}' already has priority {priority}[/yellow]") + raise typer.Exit(0) + + old_priority = normalize_priority(raw_priority) + + # Update priority + manager.registry.update(extension_id, {"priority": priority}) + + console.print(f"[green]✓[/green] Extension '{display_name}' priority changed: {old_priority} → {priority}") + console.print("\n[dim]Lower priority = higher precedence in template resolution[/dim]") + + def main(): app() diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 0dfd40b7..984ca83d 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -41,6 +41,26 @@ class CompatibilityError(ExtensionError): pass +def normalize_priority(value: Any, default: int = 10) -> int: + """Normalize a stored priority value for sorting and display. + + Corrupted registry data may contain missing, non-numeric, or non-positive + values. In those cases, fall back to the default priority. + + Args: + value: Priority value to normalize (may be int, str, None, etc.) + default: Default priority to use for invalid values (default: 10) + + Returns: + Normalized priority as positive integer (>= 1) + """ + try: + priority = int(value) + except (TypeError, ValueError): + return default + return priority if priority >= 1 else default + + @dataclass class CatalogEntry: """Represents a single catalog entry in the catalog stack.""" @@ -251,6 +271,9 @@ class ExtensionRegistry: raise KeyError(f"Extension '{extension_id}' is not installed") # Merge new metadata with existing, preserving original installed_at existing = self.data["extensions"][extension_id] + # Handle corrupted registry entries (e.g., string/list instead of dict) + if not isinstance(existing, dict): + existing = {} # Merge: existing fields preserved, new fields override merged = {**existing, **metadata} # Always preserve original installed_at based on key existence, not truthiness, @@ -324,6 +347,32 @@ class ExtensionRegistry: """ return extension_id in self.data["extensions"] + def list_by_priority(self) -> List[tuple]: + """Get all installed extensions sorted by priority. + + Lower priority number = higher precedence (checked first). + Extensions with equal priority are sorted alphabetically by ID + for deterministic ordering. + + Returns: + List of (extension_id, metadata_copy) tuples sorted by priority. + Metadata is deep-copied to prevent accidental mutation. + """ + extensions = self.data.get("extensions", {}) or {} + if not isinstance(extensions, dict): + extensions = {} + sortable_extensions = [] + for ext_id, meta in extensions.items(): + if not isinstance(meta, dict): + continue + metadata_copy = copy.deepcopy(meta) + metadata_copy["priority"] = normalize_priority(metadata_copy.get("priority", 10)) + sortable_extensions.append((ext_id, metadata_copy)) + return sorted( + sortable_extensions, + key=lambda item: (item[1]["priority"], item[0]), + ) + class ExtensionManager: """Manages extension lifecycle: installation, removal, updates.""" @@ -440,7 +489,8 @@ class ExtensionManager: self, source_dir: Path, speckit_version: str, - register_commands: bool = True + register_commands: bool = True, + priority: int = 10, ) -> ExtensionManifest: """Install extension from a local directory. @@ -448,14 +498,19 @@ class ExtensionManager: source_dir: Path to extension directory speckit_version: Current spec-kit version register_commands: If True, register commands with AI agents + priority: Resolution priority (lower = higher precedence, default 10) Returns: Installed extension manifest Raises: - ValidationError: If manifest is invalid + ValidationError: If manifest is invalid or priority is invalid CompatibilityError: If extension is incompatible """ + # Validate priority + if priority < 1: + raise ValidationError("Priority must be a positive integer (1 or higher)") + # Load and validate manifest manifest_path = source_dir / "extension.yml" manifest = ExtensionManifest(manifest_path) @@ -497,6 +552,7 @@ class ExtensionManager: "source": "local", "manifest_hash": manifest.get_hash(), "enabled": True, + "priority": priority, "registered_commands": registered_commands }) @@ -505,21 +561,27 @@ class ExtensionManager: def install_from_zip( self, zip_path: Path, - speckit_version: str + speckit_version: str, + priority: int = 10, ) -> ExtensionManifest: """Install extension from ZIP file. Args: zip_path: Path to extension ZIP file speckit_version: Current spec-kit version + priority: Resolution priority (lower = higher precedence, default 10) Returns: Installed extension manifest Raises: - ValidationError: If manifest is invalid + ValidationError: If manifest is invalid or priority is invalid CompatibilityError: If extension is incompatible """ + # Validate priority early + if priority < 1: + raise ValidationError("Priority must be a positive integer (1 or higher)") + with tempfile.TemporaryDirectory() as tmpdir: temp_path = Path(tmpdir) @@ -554,7 +616,7 @@ class ExtensionManager: raise ValidationError("No extension.yml found in ZIP file") # Install from extracted directory - return self.install_from_directory(extension_dir, speckit_version) + return self.install_from_directory(extension_dir, speckit_version, priority=priority) def remove(self, extension_id: str, keep_config: bool = False) -> bool: """Remove an installed extension. @@ -632,6 +694,9 @@ class ExtensionManager: result = [] for ext_id, metadata in self.registry.list().items(): + # Ensure metadata is a dictionary to avoid AttributeError when using .get() + if not isinstance(metadata, dict): + metadata = {} ext_dir = self.extensions_dir / ext_id manifest_path = ext_dir / "extension.yml" @@ -643,6 +708,7 @@ class ExtensionManager: "version": metadata.get("version", "unknown"), "description": manifest.description, "enabled": metadata.get("enabled", True), + "priority": normalize_priority(metadata.get("priority")), "installed_at": metadata.get("installed_at"), "command_count": len(manifest.commands), "hook_count": len(manifest.hooks) @@ -655,6 +721,7 @@ class ExtensionManager: "version": metadata.get("version", "unknown"), "description": "⚠️ Corrupted extension", "enabled": False, + "priority": normalize_priority(metadata.get("priority")), "installed_at": metadata.get("installed_at"), "command_count": 0, "hook_count": 0 diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 15196337..121d5961 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -7,6 +7,7 @@ Presets are self-contained, versioned collections of templates customize the Spec-Driven Development workflow. """ +import copy import json import hashlib import os @@ -23,6 +24,8 @@ import yaml from packaging import version as pkg_version from packaging.specifiers import SpecifierSet, InvalidSpecifier +from .extensions import ExtensionRegistry, normalize_priority + @dataclass class PresetCatalogEntry: @@ -271,6 +274,38 @@ class PresetRegistry: del self.data["presets"][pack_id] self._save() + def update(self, pack_id: str, updates: dict): + """Update preset metadata in registry. + + Merges the provided updates with the existing entry, preserving any + fields not specified. The installed_at timestamp is always preserved + from the original entry. + + Args: + pack_id: Preset ID + updates: Partial metadata to merge into existing metadata + + Raises: + KeyError: If preset is not installed + """ + if pack_id not in self.data["presets"]: + raise KeyError(f"Preset '{pack_id}' not found in registry") + existing = self.data["presets"][pack_id] + # Handle corrupted registry entries (e.g., string/list instead of dict) + if not isinstance(existing, dict): + existing = {} + # Merge: existing fields preserved, new fields override + merged = {**existing, **updates} + # Always preserve original installed_at based on key existence, not truthiness, + # to handle cases where the field exists but may be falsy (legacy/corruption) + if "installed_at" in existing: + merged["installed_at"] = existing["installed_at"] + else: + # If not present in existing, explicitly remove from merged if caller provided it + merged.pop("installed_at", None) + self.data["presets"][pack_id] = merged + self._save() + def get(self, pack_id: str) -> Optional[dict]: """Get preset metadata from registry. @@ -294,14 +329,26 @@ class PresetRegistry: """Get all installed presets sorted by priority. Lower priority number = higher precedence (checked first). + Presets with equal priority are sorted alphabetically by ID + for deterministic ordering. Returns: - List of (pack_id, metadata) tuples sorted by priority + List of (pack_id, metadata_copy) tuples sorted by priority. + Metadata is deep-copied to prevent accidental mutation. """ - packs = self.data["presets"] + packs = self.data.get("presets", {}) or {} + if not isinstance(packs, dict): + packs = {} + sortable_packs = [] + for pack_id, meta in packs.items(): + if not isinstance(meta, dict): + continue + metadata_copy = copy.deepcopy(meta) + metadata_copy["priority"] = normalize_priority(metadata_copy.get("priority", 10)) + sortable_packs.append((pack_id, metadata_copy)) return sorted( - packs.items(), - key=lambda item: item[1].get("priority", 10), + sortable_packs, + key=lambda item: (item[1]["priority"], item[0]), ) def is_installed(self, pack_id: str) -> bool: @@ -680,9 +727,13 @@ class PresetManager: Installed preset manifest Raises: - PresetValidationError: If manifest is invalid + PresetValidationError: If manifest is invalid or priority is invalid PresetCompatibilityError: If pack is incompatible """ + # Validate priority + if priority < 1: + raise PresetValidationError("Priority must be a positive integer (1 or higher)") + manifest_path = source_dir / "preset.yml" manifest = PresetManifest(manifest_path) @@ -729,14 +780,19 @@ class PresetManager: Args: zip_path: Path to preset ZIP file speckit_version: Current spec-kit version + priority: Resolution priority (lower = higher precedence, default 10) Returns: Installed preset manifest Raises: - PresetValidationError: If manifest is invalid + PresetValidationError: If manifest is invalid or priority is invalid PresetCompatibilityError: If pack is incompatible """ + # Validate priority early + if priority < 1: + raise PresetValidationError("Priority must be a positive integer (1 or higher)") + with tempfile.TemporaryDirectory() as tmpdir: temp_path = Path(tmpdir) @@ -808,6 +864,9 @@ class PresetManager: result = [] for pack_id, metadata in self.registry.list().items(): + # Ensure metadata is a dictionary to avoid AttributeError when using .get() + if not isinstance(metadata, dict): + metadata = {} pack_dir = self.presets_dir / pack_id manifest_path = pack_dir / "preset.yml" @@ -816,13 +875,13 @@ class PresetManager: result.append({ "id": pack_id, "name": manifest.name, - "version": metadata["version"], + "version": metadata.get("version", manifest.version), "description": manifest.description, "enabled": metadata.get("enabled", True), "installed_at": metadata.get("installed_at"), "template_count": len(manifest.templates), "tags": manifest.tags, - "priority": metadata.get("priority", 10), + "priority": normalize_priority(metadata.get("priority")), }) except PresetValidationError: result.append({ @@ -834,7 +893,7 @@ class PresetManager: "installed_at": metadata.get("installed_at"), "template_count": 0, "tags": [], - "priority": metadata.get("priority", 10), + "priority": normalize_priority(metadata.get("priority")), }) return result @@ -1393,6 +1452,40 @@ class PresetResolver: self.overrides_dir = self.templates_dir / "overrides" self.extensions_dir = project_root / ".specify" / "extensions" + def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]: + """Build unified list of registered and unregistered extensions sorted by priority. + + Registered extensions use their stored priority; unregistered directories + get implicit priority=10. Results are sorted by (priority, ext_id) for + deterministic ordering. + + Returns: + List of (priority, ext_id, metadata_or_none) tuples sorted by priority. + """ + if not self.extensions_dir.exists(): + return [] + + registry = ExtensionRegistry(self.extensions_dir) + registered_extensions = registry.list_by_priority() + registered_extension_ids = {ext_id for ext_id, _ in registered_extensions} + + all_extensions: list[tuple[int, str, dict | None]] = [] + + for ext_id, metadata in registered_extensions: + priority = normalize_priority(metadata.get("priority") if metadata else None) + all_extensions.append((priority, ext_id, metadata)) + + # Add unregistered directories with implicit priority=10 + for ext_dir in self.extensions_dir.iterdir(): + if not ext_dir.is_dir() or ext_dir.name.startswith("."): + continue + if ext_dir.name not in registered_extension_ids: + all_extensions.append((10, ext_dir.name, None)) + + # Sort by (priority, ext_id) for deterministic ordering + all_extensions.sort(key=lambda x: (x[0], x[1])) + return all_extensions + def resolve( self, template_name: str, @@ -1445,18 +1538,18 @@ class PresetResolver: if candidate.exists(): return candidate - # Priority 3: Extension-provided templates - if self.extensions_dir.exists(): - for ext_dir in sorted(self.extensions_dir.iterdir()): - if not ext_dir.is_dir() or ext_dir.name.startswith("."): - continue - for subdir in subdirs: - if subdir: - candidate = ext_dir / subdir / f"{template_name}{ext}" - else: - candidate = ext_dir / "templates" / f"{template_name}{ext}" - if candidate.exists(): - return candidate + # Priority 3: Extension-provided templates (sorted by priority — lower number wins) + for _priority, ext_id, _metadata in self._get_all_extensions_by_priority(): + ext_dir = self.extensions_dir / ext_id + if not ext_dir.is_dir(): + continue + for subdir in subdirs: + if subdir: + candidate = ext_dir / subdir / f"{template_name}{ext}" + else: + candidate = ext_dir / f"{template_name}{ext}" + if candidate.exists(): + return candidate # Priority 4: Core templates if template_type == "template": @@ -1514,17 +1607,24 @@ class PresetResolver: except ValueError: continue - if self.extensions_dir.exists(): - for ext_dir in sorted(self.extensions_dir.iterdir()): - if not ext_dir.is_dir() or ext_dir.name.startswith("."): - continue - try: - resolved.relative_to(ext_dir) + for _priority, ext_id, ext_meta in self._get_all_extensions_by_priority(): + ext_dir = self.extensions_dir / ext_id + if not ext_dir.is_dir(): + continue + try: + resolved.relative_to(ext_dir) + if ext_meta: + version = ext_meta.get("version", "?") return { "path": resolved_str, - "source": f"extension:{ext_dir.name}", + "source": f"extension:{ext_id} v{version}", } - except ValueError: - continue + else: + return { + "path": resolved_str, + "source": f"extension:{ext_id} (unregistered)", + } + except ValueError: + continue return {"path": resolved_str, "source": "core"} diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 61a3e1c9..c87ba5b5 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -26,6 +26,7 @@ from specify_cli.extensions import ( ExtensionError, ValidationError, CompatibilityError, + normalize_priority, version_satisfies, ) @@ -121,6 +122,57 @@ def project_dir(temp_dir): return proj_dir +# ===== normalize_priority Tests ===== + +class TestNormalizePriority: + """Test normalize_priority helper function.""" + + def test_valid_integer(self): + """Test with valid integer priority.""" + assert normalize_priority(5) == 5 + assert normalize_priority(1) == 1 + assert normalize_priority(100) == 100 + + def test_valid_string_number(self): + """Test with string that can be converted to int.""" + assert normalize_priority("5") == 5 + assert normalize_priority("10") == 10 + + def test_zero_returns_default(self): + """Test that zero priority returns default.""" + assert normalize_priority(0) == 10 + assert normalize_priority(0, default=5) == 5 + + def test_negative_returns_default(self): + """Test that negative priority returns default.""" + assert normalize_priority(-1) == 10 + assert normalize_priority(-100, default=5) == 5 + + def test_none_returns_default(self): + """Test that None returns default.""" + assert normalize_priority(None) == 10 + assert normalize_priority(None, default=5) == 5 + + def test_invalid_string_returns_default(self): + """Test that non-numeric string returns default.""" + assert normalize_priority("invalid") == 10 + assert normalize_priority("abc", default=5) == 5 + + def test_float_truncates(self): + """Test that float is truncated to int.""" + assert normalize_priority(5.9) == 5 + assert normalize_priority(3.1) == 3 + + def test_empty_string_returns_default(self): + """Test that empty string returns default.""" + assert normalize_priority("") == 10 + + def test_custom_default(self): + """Test custom default value.""" + assert normalize_priority(None, default=20) == 20 + assert normalize_priority("invalid", default=1) == 1 + + # ===== ExtensionManifest Tests ===== class TestExtensionManifest: @@ -2363,3 +2415,378 @@ class TestExtensionListCLI: # Verify name and version are also shown assert "Test Extension" in result.output assert "1.0.0" in result.output + + +class TestExtensionPriority: + """Test extension priority-based resolution.""" + + def test_list_by_priority_empty(self, temp_dir): + """Test list_by_priority on empty registry.""" + extensions_dir = temp_dir / "extensions" + extensions_dir.mkdir() + + registry = ExtensionRegistry(extensions_dir) + result = registry.list_by_priority() + + assert result == [] + + def test_list_by_priority_single(self, temp_dir): + """Test list_by_priority with single extension.""" + extensions_dir = temp_dir / "extensions" + extensions_dir.mkdir() + + registry = ExtensionRegistry(extensions_dir) + registry.add("test-ext", {"version": "1.0.0", "priority": 5}) + + result = registry.list_by_priority() + + assert len(result) == 1 + assert result[0][0] == "test-ext" + assert result[0][1]["priority"] == 5 + + def test_list_by_priority_ordering(self, temp_dir): + """Test list_by_priority returns extensions sorted by priority.""" + extensions_dir = temp_dir / "extensions" + extensions_dir.mkdir() + + registry = ExtensionRegistry(extensions_dir) + # Add in non-priority order + registry.add("ext-low", {"version": "1.0.0", "priority": 20}) + registry.add("ext-high", {"version": "1.0.0", "priority": 1}) + registry.add("ext-mid", {"version": "1.0.0", "priority": 10}) + + result = registry.list_by_priority() + + assert len(result) == 3 + # Lower priority number = higher precedence (first) + assert result[0][0] == "ext-high" + assert result[1][0] == "ext-mid" + assert result[2][0] == "ext-low" + + def test_list_by_priority_default(self, temp_dir): + """Test list_by_priority uses default priority of 10.""" + extensions_dir = temp_dir / "extensions" + extensions_dir.mkdir() + + registry = ExtensionRegistry(extensions_dir) + # Add without explicit priority + registry.add("ext-default", {"version": "1.0.0"}) + registry.add("ext-high", {"version": "1.0.0", "priority": 1}) + registry.add("ext-low", {"version": "1.0.0", "priority": 20}) + + result = registry.list_by_priority() + + assert len(result) == 3 + # ext-high (1), ext-default (10), ext-low (20) + assert result[0][0] == "ext-high" + assert result[1][0] == "ext-default" + assert result[2][0] == "ext-low" + + def test_list_by_priority_invalid_priority_defaults(self, temp_dir): + """Malformed priority values fall back to the default priority.""" + extensions_dir = temp_dir / "extensions" + extensions_dir.mkdir() + + registry = ExtensionRegistry(extensions_dir) + registry.add("ext-high", {"version": "1.0.0", "priority": 1}) + registry.data["extensions"]["ext-invalid"] = { + "version": "1.0.0", + "priority": "high", + } + registry._save() + + result = registry.list_by_priority() + + assert [item[0] for item in result] == ["ext-high", "ext-invalid"] + assert result[1][1]["priority"] == 10 + + def test_install_with_priority(self, extension_dir, project_dir): + """Test that install_from_directory stores priority.""" + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False, priority=5) + + metadata = manager.registry.get("test-ext") + assert metadata["priority"] == 5 + + def test_install_default_priority(self, extension_dir, project_dir): + """Test that install_from_directory uses default priority of 10.""" + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + metadata = manager.registry.get("test-ext") + assert metadata["priority"] == 10 + + def test_list_installed_includes_priority(self, extension_dir, project_dir): + """Test that list_installed includes priority in returned data.""" + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False, priority=3) + + installed = manager.list_installed() + + assert len(installed) == 1 + assert installed[0]["priority"] == 3 + + def test_priority_preserved_on_update(self, temp_dir): + """Test that registry update preserves priority.""" + extensions_dir = temp_dir / "extensions" + extensions_dir.mkdir() + + registry = ExtensionRegistry(extensions_dir) + registry.add("test-ext", {"version": "1.0.0", "priority": 5, "enabled": True}) + + # Update with new metadata (no priority specified) + registry.update("test-ext", {"enabled": False}) + + updated = registry.get("test-ext") + assert updated["priority"] == 5 # Preserved + assert updated["enabled"] is False # Updated + + def test_resolve_uses_unregistered_extension_dirs_when_registry_partially_corrupted(self, project_dir): + """Resolution scans unregistered extension dirs after valid registry entries.""" + extensions_dir = project_dir / ".specify" / "extensions" + + valid_dir = extensions_dir / "valid-ext" / "templates" + valid_dir.mkdir(parents=True) + (valid_dir / "other-template.md").write_text("# Valid\n") + + broken_dir = extensions_dir / "broken-ext" / "templates" + broken_dir.mkdir(parents=True) + (broken_dir / "target-template.md").write_text("# Broken Target\n") + + registry = ExtensionRegistry(extensions_dir) + registry.add("valid-ext", {"version": "1.0.0", "priority": 10}) + registry.data["extensions"]["broken-ext"] = "corrupted" + registry._save() + + from specify_cli.presets import PresetResolver + + resolver = PresetResolver(project_dir) + resolved = resolver.resolve("target-template") + sourced = resolver.resolve_with_source("target-template") + + assert resolved is not None + assert resolved.name == "target-template.md" + assert "Broken Target" in resolved.read_text() + assert sourced is not None + assert sourced["source"] == "extension:broken-ext (unregistered)" + + +class TestExtensionPriorityCLI: + """Test extension priority CLI integration.""" + + def test_add_with_priority_option(self, extension_dir, project_dir): + """Test extension add command with --priority option.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, [ + "extension", "add", str(extension_dir), "--dev", "--priority", "3" + ]) + + assert result.exit_code == 0, result.output + + manager = ExtensionManager(project_dir) + metadata = manager.registry.get("test-ext") + assert metadata["priority"] == 3 + + def test_list_shows_priority(self, extension_dir, project_dir): + """Test extension list shows priority.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Install extension with priority + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False, priority=7) + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["extension", "list"]) + + assert result.exit_code == 0, result.output + assert "Priority: 7" in result.output + + def test_set_priority_changes_priority(self, extension_dir, project_dir): + """Test set-priority command changes extension priority.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Install extension with default priority + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + # Verify default priority + assert manager.registry.get("test-ext")["priority"] == 10 + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["extension", "set-priority", "test-ext", "5"]) + + assert result.exit_code == 0, result.output + assert "priority changed: 10 → 5" in result.output + + # Reload registry to see updated value + manager2 = ExtensionManager(project_dir) + assert manager2.registry.get("test-ext")["priority"] == 5 + + def test_set_priority_same_value_no_change(self, extension_dir, project_dir): + """Test set-priority with same value shows already set message.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Install extension with priority 5 + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False, priority=5) + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["extension", "set-priority", "test-ext", "5"]) + + assert result.exit_code == 0, result.output + assert "already has priority 5" in result.output + + def test_set_priority_invalid_value(self, extension_dir, project_dir): + """Test set-priority rejects invalid priority values.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Install extension + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["extension", "set-priority", "test-ext", "0"]) + + assert result.exit_code == 1, result.output + assert "Priority must be a positive integer" in result.output + + def test_set_priority_not_installed(self, project_dir): + """Test set-priority fails for non-installed extension.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Ensure .specify exists + (project_dir / ".specify").mkdir(parents=True, exist_ok=True) + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["extension", "set-priority", "nonexistent", "5"]) + + assert result.exit_code == 1, result.output + assert "not installed" in result.output.lower() or "no extensions installed" in result.output.lower() + + def test_set_priority_by_display_name(self, extension_dir, project_dir): + """Test set-priority works with extension display name.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Install extension + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + # Use display name "Test Extension" instead of ID "test-ext" + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["extension", "set-priority", "Test Extension", "3"]) + + assert result.exit_code == 0, result.output + assert "priority changed" in result.output + + # Reload registry to see updated value + manager2 = ExtensionManager(project_dir) + assert manager2.registry.get("test-ext")["priority"] == 3 + + +class TestExtensionPriorityBackwardsCompatibility: + """Test backwards compatibility for extensions installed before priority feature.""" + + def test_legacy_extension_without_priority_field(self, temp_dir): + """Extensions installed before priority feature should default to 10.""" + extensions_dir = temp_dir / "extensions" + extensions_dir.mkdir() + + # Simulate legacy registry entry without priority field + registry = ExtensionRegistry(extensions_dir) + registry.data["extensions"]["legacy-ext"] = { + "version": "1.0.0", + "source": "local", + "enabled": True, + "installed_at": "2025-01-01T00:00:00Z", + # No "priority" field - simulates pre-feature extension + } + registry._save() + + # Reload registry + registry2 = ExtensionRegistry(extensions_dir) + + # list_by_priority should use default of 10 + result = registry2.list_by_priority() + assert len(result) == 1 + assert result[0][0] == "legacy-ext" + # Priority defaults to 10 and is normalized in returned metadata + assert result[0][1]["priority"] == 10 + + def test_legacy_extension_in_list_installed(self, extension_dir, project_dir): + """list_installed returns priority=10 for legacy extensions without priority field.""" + manager = ExtensionManager(project_dir) + + # Install extension normally + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + + # Manually remove priority to simulate legacy extension + ext_data = manager.registry.data["extensions"]["test-ext"] + del ext_data["priority"] + manager.registry._save() + + # list_installed should still return priority=10 + installed = manager.list_installed() + assert len(installed) == 1 + assert installed[0]["priority"] == 10 + + def test_mixed_legacy_and_new_extensions_ordering(self, temp_dir): + """Legacy extensions (no priority) sort with default=10 among prioritized extensions.""" + extensions_dir = temp_dir / "extensions" + extensions_dir.mkdir() + + registry = ExtensionRegistry(extensions_dir) + + # Add extension with explicit priority=5 + registry.add("ext-with-priority", {"version": "1.0.0", "priority": 5}) + + # Add legacy extension without priority (manually) + registry.data["extensions"]["legacy-ext"] = { + "version": "1.0.0", + "source": "local", + "enabled": True, + # No priority field + } + registry._save() + + # Add extension with priority=15 + registry.add("ext-low-priority", {"version": "1.0.0", "priority": 15}) + + # Reload and check ordering + registry2 = ExtensionRegistry(extensions_dir) + result = registry2.list_by_priority() + + assert len(result) == 3 + # Order: ext-with-priority (5), legacy-ext (defaults to 10), ext-low-priority (15) + assert result[0][0] == "ext-with-priority" + assert result[1][0] == "legacy-ext" + assert result[2][0] == "ext-low-priority" diff --git a/tests/test_presets.py b/tests/test_presets.py index 3ad70c6d..b6fe81d5 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -32,6 +32,7 @@ from specify_cli.presets import ( PresetCompatibilityError, VALID_PRESET_TEMPLATE_TYPES, ) +from specify_cli.extensions import ExtensionRegistry # ===== Fixtures ===== @@ -573,6 +574,24 @@ class TestRegistryPriority: assert sorted_packs[0][0] == "pack-b" assert sorted_packs[1][0] == "pack-a" + def test_list_by_priority_invalid_priority_defaults(self, temp_dir): + """Malformed priority values fall back to the default priority.""" + packs_dir = temp_dir / "packs" + packs_dir.mkdir() + registry = PresetRegistry(packs_dir) + + registry.add("pack-high", {"version": "1.0.0", "priority": 1}) + registry.data["presets"]["pack-invalid"] = { + "version": "1.0.0", + "priority": "high", + } + registry._save() + + sorted_packs = registry.list_by_priority() + + assert [item[0] for item in sorted_packs] == ["pack-high", "pack-invalid"] + assert sorted_packs[1][1]["priority"] == 10 + # ===== PresetResolver Tests ===== @@ -678,6 +697,11 @@ class TestPresetResolver: ext_template = ext_templates_dir / "custom-template.md" ext_template.write_text("# Extension Custom Template\n") + # Register extension in registry + extensions_dir = project_dir / ".specify" / "extensions" + ext_registry = ExtensionRegistry(extensions_dir) + ext_registry.add("my-ext", {"version": "1.0.0", "priority": 10}) + resolver = PresetResolver(project_dir) result = resolver.resolve("custom-template") assert result is not None @@ -741,10 +765,15 @@ class TestPresetResolver: ext_template = ext_templates_dir / "unique-template.md" ext_template.write_text("# Unique\n") + # Register extension in registry + extensions_dir = project_dir / ".specify" / "extensions" + ext_registry = ExtensionRegistry(extensions_dir) + ext_registry.add("my-ext", {"version": "1.0.0", "priority": 10}) + resolver = PresetResolver(project_dir) result = resolver.resolve_with_source("unique-template") assert result is not None - assert result["source"] == "extension:my-ext" + assert result["source"] == "extension:my-ext v1.0.0" def test_resolve_with_source_not_found(self, project_dir): """Test resolve_with_source for nonexistent template.""" @@ -765,6 +794,104 @@ class TestPresetResolver: assert result is None +class TestExtensionPriorityResolution: + """Test extension priority resolution with registered and unregistered extensions.""" + + def test_unregistered_beats_registered_with_lower_precedence(self, project_dir): + """Unregistered extension (implicit priority 10) beats registered with priority 20.""" + extensions_dir = project_dir / ".specify" / "extensions" + extensions_dir.mkdir(parents=True, exist_ok=True) + + # Create registered extension with priority 20 (lower precedence than 10) + registered_dir = extensions_dir / "registered-ext" + (registered_dir / "templates").mkdir(parents=True) + (registered_dir / "templates" / "test-template.md").write_text("# From Registered\n") + + ext_registry = ExtensionRegistry(extensions_dir) + ext_registry.add("registered-ext", {"version": "1.0.0", "priority": 20}) + + # Create unregistered extension directory (implicit priority 10) + unregistered_dir = extensions_dir / "unregistered-ext" + (unregistered_dir / "templates").mkdir(parents=True) + (unregistered_dir / "templates" / "test-template.md").write_text("# From Unregistered\n") + + # Unregistered (priority 10) should beat registered (priority 20) + resolver = PresetResolver(project_dir) + result = resolver.resolve("test-template") + assert result is not None + assert "From Unregistered" in result.read_text() + + def test_registered_with_higher_precedence_beats_unregistered(self, project_dir): + """Registered extension with priority 5 beats unregistered (implicit priority 10).""" + extensions_dir = project_dir / ".specify" / "extensions" + extensions_dir.mkdir(parents=True, exist_ok=True) + + # Create registered extension with priority 5 (higher precedence than 10) + registered_dir = extensions_dir / "registered-ext" + (registered_dir / "templates").mkdir(parents=True) + (registered_dir / "templates" / "test-template.md").write_text("# From Registered\n") + + ext_registry = ExtensionRegistry(extensions_dir) + ext_registry.add("registered-ext", {"version": "1.0.0", "priority": 5}) + + # Create unregistered extension directory (implicit priority 10) + unregistered_dir = extensions_dir / "unregistered-ext" + (unregistered_dir / "templates").mkdir(parents=True) + (unregistered_dir / "templates" / "test-template.md").write_text("# From Unregistered\n") + + # Registered (priority 5) should beat unregistered (priority 10) + resolver = PresetResolver(project_dir) + result = resolver.resolve("test-template") + assert result is not None + assert "From Registered" in result.read_text() + + def test_unregistered_attribution_with_priority_ordering(self, project_dir): + """Test resolve_with_source correctly attributes unregistered extension.""" + extensions_dir = project_dir / ".specify" / "extensions" + extensions_dir.mkdir(parents=True, exist_ok=True) + + # Create registered extension with priority 20 + registered_dir = extensions_dir / "registered-ext" + (registered_dir / "templates").mkdir(parents=True) + (registered_dir / "templates" / "test-template.md").write_text("# From Registered\n") + + ext_registry = ExtensionRegistry(extensions_dir) + ext_registry.add("registered-ext", {"version": "1.0.0", "priority": 20}) + + # Create unregistered extension (implicit priority 10) + unregistered_dir = extensions_dir / "unregistered-ext" + (unregistered_dir / "templates").mkdir(parents=True) + (unregistered_dir / "templates" / "test-template.md").write_text("# From Unregistered\n") + + # Attribution should show unregistered extension + resolver = PresetResolver(project_dir) + result = resolver.resolve_with_source("test-template") + assert result is not None + assert "unregistered-ext" in result["source"] + assert "(unregistered)" in result["source"] + + def test_same_priority_sorted_alphabetically(self, project_dir): + """Extensions with same priority are sorted alphabetically by ID.""" + extensions_dir = project_dir / ".specify" / "extensions" + extensions_dir.mkdir(parents=True, exist_ok=True) + + # Create two unregistered extensions (both implicit priority 10) + # "aaa-ext" should come before "zzz-ext" alphabetically + zzz_dir = extensions_dir / "zzz-ext" + (zzz_dir / "templates").mkdir(parents=True) + (zzz_dir / "templates" / "test-template.md").write_text("# From ZZZ\n") + + aaa_dir = extensions_dir / "aaa-ext" + (aaa_dir / "templates").mkdir(parents=True) + (aaa_dir / "templates" / "test-template.md").write_text("# From AAA\n") + + # AAA should win due to alphabetical ordering at same priority + resolver = PresetResolver(project_dir) + result = resolver.resolve("test-template") + assert result is not None + assert "From AAA" in result.read_text() + + # ===== PresetCatalog Tests ===== @@ -979,8 +1106,13 @@ class TestIntegration: ext_templates_dir.mkdir(parents=True) (ext_templates_dir / "spec-template.md").write_text("# Extension\n") + # Register extension in registry + extensions_dir = project_dir / ".specify" / "extensions" + ext_registry = ExtensionRegistry(extensions_dir) + ext_registry.add("my-ext", {"version": "1.0.0", "priority": 10}) + result = resolver.resolve_with_source("spec-template") - assert result["source"] == "extension:my-ext" + assert result["source"] == "extension:my-ext v1.0.0" # Install pack — should win over extension manager = PresetManager(project_dir) @@ -1710,3 +1842,162 @@ class TestPresetSkills: metadata = manager.registry.get("self-test") assert metadata.get("registered_skills", []) == [] + + +class TestPresetSetPriority: + """Test preset set-priority CLI command.""" + + def test_set_priority_changes_priority(self, project_dir, pack_dir): + """Test set-priority command changes preset priority.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Install preset with default priority + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + + # Verify default priority + assert manager.registry.get("test-pack")["priority"] == 10 + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["preset", "set-priority", "test-pack", "5"]) + + assert result.exit_code == 0, result.output + assert "priority changed: 10 → 5" in result.output + + # Reload registry to see updated value + manager2 = PresetManager(project_dir) + assert manager2.registry.get("test-pack")["priority"] == 5 + + def test_set_priority_same_value_no_change(self, project_dir, pack_dir): + """Test set-priority with same value shows already set message.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Install preset with priority 5 + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5", priority=5) + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["preset", "set-priority", "test-pack", "5"]) + + assert result.exit_code == 0, result.output + assert "already has priority 5" in result.output + + def test_set_priority_invalid_value(self, project_dir, pack_dir): + """Test set-priority rejects invalid priority values.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Install preset + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["preset", "set-priority", "test-pack", "0"]) + + assert result.exit_code == 1, result.output + assert "Priority must be a positive integer" in result.output + + def test_set_priority_not_installed(self, project_dir): + """Test set-priority fails for non-installed preset.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["preset", "set-priority", "nonexistent", "5"]) + + assert result.exit_code == 1, result.output + assert "not installed" in result.output.lower() + + +class TestPresetPriorityBackwardsCompatibility: + """Test backwards compatibility for presets installed before priority feature.""" + + def test_legacy_preset_without_priority_field(self, temp_dir): + """Presets installed before priority feature should default to 10.""" + presets_dir = temp_dir / ".specify" / "presets" + presets_dir.mkdir(parents=True) + + # Simulate legacy registry entry without priority field + registry = PresetRegistry(presets_dir) + registry.data["presets"]["legacy-pack"] = { + "version": "1.0.0", + "source": "local", + "enabled": True, + "installed_at": "2025-01-01T00:00:00Z", + # No "priority" field - simulates pre-feature preset + } + registry._save() + + # Reload registry + registry2 = PresetRegistry(presets_dir) + + # list_by_priority should use default of 10 + result = registry2.list_by_priority() + assert len(result) == 1 + assert result[0][0] == "legacy-pack" + # Priority defaults to 10 and is normalized in returned metadata + assert result[0][1]["priority"] == 10 + + def test_legacy_preset_in_list_installed(self, project_dir, pack_dir): + """list_installed returns priority=10 for legacy presets without priority field.""" + manager = PresetManager(project_dir) + + # Install preset normally + manager.install_from_directory(pack_dir, "0.1.5") + + # Manually remove priority to simulate legacy preset + pack_data = manager.registry.data["presets"]["test-pack"] + del pack_data["priority"] + manager.registry._save() + + # list_installed should still return priority=10 + installed = manager.list_installed() + assert len(installed) == 1 + assert installed[0]["priority"] == 10 + + def test_mixed_legacy_and_new_presets_ordering(self, temp_dir): + """Legacy presets (no priority) sort with default=10 among prioritized presets.""" + presets_dir = temp_dir / ".specify" / "presets" + presets_dir.mkdir(parents=True) + + registry = PresetRegistry(presets_dir) + + # Add preset with explicit priority=5 + registry.add("pack-with-priority", {"version": "1.0.0", "priority": 5}) + + # Add legacy preset without priority (manually) + registry.data["presets"]["legacy-pack"] = { + "version": "1.0.0", + "source": "local", + "enabled": True, + # No priority field + } + + # Add another preset with priority=15 + registry.add("low-priority-pack", {"version": "1.0.0", "priority": 15}) + registry._save() + + # Reload and check ordering + registry2 = PresetRegistry(presets_dir) + sorted_presets = registry2.list_by_priority() + + # Should be: pack-with-priority (5), legacy-pack (default 10), low-priority-pack (15) + assert [p[0] for p in sorted_presets] == [ + "pack-with-priority", + "legacy-pack", + "low-priority-pack", + ]