diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 6242dc2c05..aa3330f513 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -44,7 +44,11 @@ ) from .._init_options import is_ai_skills_enabled from .._invocation_style import is_dollar_skills_agent, is_slash_skills_agent -from .._utils import dump_frontmatter, relative_extension_path_violation, version_satisfies +from .._utils import ( + dump_frontmatter, + relative_extension_path_violation, + version_satisfies, +) from ..catalogs import CatalogEntry as BaseCatalogEntry from ..catalogs import CatalogStackBase from ..integration_state import ( @@ -161,7 +165,12 @@ def _fsync_directory(path: Path) -> None: except (AttributeError, NotImplementedError): return except OSError as exc2: - if exc2.errno in {errno.ENOTSUP, errno.EOPNOTSUPP, errno.EINVAL, errno.EBADF}: + if exc2.errno in { + errno.ENOTSUP, + errno.EOPNOTSUPP, + errno.EINVAL, + errno.EBADF, + }: return raise try: @@ -400,6 +409,7 @@ def _validate(self): raise ValidationError("Invalid hooks: expected a mapping") if "events" in self.data: from ..events import validate_events + validate_events(self.data) has_commands = bool(commands) @@ -408,13 +418,21 @@ def _validate(self): has_templates = bool(templates) has_scripts = bool(scripts) - if not has_commands and not has_hooks and not has_events and not has_templates and not has_scripts: + if ( + not has_commands + and not has_hooks + and not has_events + and not has_templates + and not has_scripts + ): raise ValidationError( "Extension must provide at least one command, hook, or event " "(or a declared template/script)" ) - self._validate_provided_artifacts(templates, section="templates", singular="template") + self._validate_provided_artifacts( + templates, section="templates", singular="template" + ) self._validate_provided_artifacts(scripts, section="scripts", singular="script") # Validate hook values (if present). @@ -477,7 +495,11 @@ def _validate(self): cmd_file = cmd["file"] reason = relative_extension_path_violation(cmd_file) if reason: - label = repr(cmd_file) if isinstance(cmd_file, str) else f"for command '{cmd.get('name')}'" + label = ( + repr(cmd_file) + if isinstance(cmd_file, str) + else f"for command '{cmd.get('name')}'" + ) raise ValidationError(f"Invalid command 'file' {label}: {reason}") # Validate command name format @@ -577,7 +599,9 @@ def _validate(self): ) @staticmethod - def _validate_provided_artifacts(entries: List[Any], section: str, singular: str) -> None: + def _validate_provided_artifacts( + entries: List[Any], section: str, singular: str + ) -> None: """Validate provides.templates / provides.scripts entries. Mirrors the shape/path-safety checks PresetManifest applies to its @@ -600,7 +624,9 @@ def _validate_provided_artifacts(entries: List[Any], section: str, singular: str f"Each entry in 'provides.{section}' must be a mapping" ) if "name" not in entry or "file" not in entry: - raise ValidationError(f"{singular.capitalize()} missing 'name' or 'file'") + raise ValidationError( + f"{singular.capitalize()} missing 'name' or 'file'" + ) name = entry["name"] if not isinstance(name, str): @@ -621,7 +647,11 @@ def _validate_provided_artifacts(entries: List[Any], section: str, singular: str file_value = entry["file"] reason = relative_extension_path_violation(file_value) if reason: - label = repr(file_value) if isinstance(file_value, str) else f"for {singular} '{name}'" + label = ( + repr(file_value) + if isinstance(file_value, str) + else f"for {singular} '{name}'" + ) raise ValidationError(f"Invalid {singular} 'file' {label}: {reason}") if "description" in entry and not isinstance(entry["description"], str): @@ -716,7 +746,9 @@ def commands(self) -> List[Dict[str, Any]]: def config(self) -> List[Dict[str, Any]]: """Get list of provided config templates, normalized to dictionaries.""" raw = self.data.get("provides", {}).get("config", []) - if not isinstance(raw, list) or not all(isinstance(entry, dict) for entry in raw): + if not isinstance(raw, list) or not all( + isinstance(entry, dict) for entry in raw + ): return [] return raw @@ -1130,9 +1162,7 @@ def _collect_manifest_command_names(manifest: ExtensionManifest) -> Dict[str, st path_reason = relative_extension_path_violation(name) if path_reason: - raise ValidationError( - f"Invalid {kind} {name!r}: {path_reason}" - ) + raise ValidationError(f"Invalid {kind} {name!r}: {path_reason}") # Enforce canonical pattern only for primary command names; # aliases are free-form to preserve community extension compat. @@ -1227,7 +1257,8 @@ def _validate_install_conflicts(self, manifest: ExtensionManifest) -> None: exclude_extension_id=manifest.id ) core_shadow_names = { - self._normalize_shadow_name(f"speckit.{name}") for name in CORE_COMMAND_NAMES + self._normalize_shadow_name(f"speckit.{name}") + for name in CORE_COMMAND_NAMES } collisions = [] @@ -1640,9 +1671,7 @@ def _replacement(match: re.Match[str]) -> str: match.group(0), agent_config.get("invoke_separator", ".") ) - return re.sub( - r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_-]*)__", _replacement, body - ) + return re.sub(r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_-]*)__", _replacement, body) for cmd_info in manifest.commands: cmd_name = cmd_info["name"] @@ -1673,9 +1702,7 @@ def _replacement(match: re.Match[str]) -> str: cache_root = extension_dir / ".specify-dev" / "extension-skills" cache_file = cache_root / skill_name / "SKILL.md" use_dev_symlink = link_outputs and not agent_config.get("dev_no_symlink") - skill_dir_preexists = ( - skill_subdir.exists() or skill_subdir.is_symlink() - ) + skill_dir_preexists = skill_subdir.exists() or skill_subdir.is_symlink() CommandRegistrar._ensure_inside(cache_file, cache_root) if selected_ai == "generic" and skill_dir_preexists: metadata = self.registry.get(manifest.id) or {} @@ -1726,7 +1753,11 @@ def _replacement(match: re.Match[str]) -> str: # before the generic placeholder/path resolution below. body = registrar.rewrite_extension_paths(body, manifest.id, extension_dir) body = registrar.resolve_skill_placeholders( - selected_ai, frontmatter, body, self.project_root, extension_id=manifest.id + selected_ai, + frontmatter, + body, + self.project_root, + extension_id=manifest.id, ) body = _resolve_command_ref_tokens(body) @@ -1911,9 +1942,7 @@ def add_candidate(candidate: Path) -> None: for cfg in AGENT_CONFIG.values(): folder = cfg.get("folder", "") if folder: - add_candidate( - self.project_root / folder.rstrip("/") / "skills" - ) + add_candidate(self.project_root / folder.rstrip("/") / "skills") add_candidate(self.project_root / DEFAULT_SKILLS_DIR) from ..integration_state import integration_setting, try_read_integration_json @@ -1955,8 +1984,7 @@ def _generic_artifact_hashes( output_dir = registration_directory(self.project_root) root = self.project_root.resolve() paths = [ - output_dir / f"{name}.md" - for name in registered_commands.get("generic", []) + output_dir / f"{name}.md" for name in registered_commands.get("generic", []) ] + [output_dir / name / "SKILL.md" for name in registered_skills] hashes = dict(previous or {}) for path in paths: @@ -1972,9 +2000,13 @@ def _generic_artifact_hashes( return hashes def _snapshot_generic_refresh_artifacts( - self, extension_id: str, manifest: Optional[ExtensionManifest], + self, + extension_id: str, + manifest: Optional[ExtensionManifest], metadata: Dict[str, Any], - *, skills_mode_active: bool, include_current_candidates: bool = True, + *, + skills_mode_active: bool, + include_current_candidates: bool = True, ) -> Dict[Path, tuple[bytes | None, str | None, bool]]: """Remember owned outputs and absent candidates for generic rollback.""" from ..integrations.generic import registration_directory @@ -1983,7 +2015,8 @@ def _snapshot_generic_refresh_artifacts( root = self.project_root.resolve() output_dir = ( registration_directory(self.project_root) - if include_current_candidates else None + if include_current_candidates + else None ) source = (self.extensions_dir / extension_id).resolve() hashes = metadata.get("generic_artifact_hashes", {}) @@ -1992,13 +2025,18 @@ def _snapshot_generic_refresh_artifacts( registered = metadata.get("registered_commands", {}) command_names = ( set(self._collect_manifest_command_names(manifest)) - if manifest is not None and not skills_mode_active else set() + if manifest is not None and not skills_mode_active + else set() ) if isinstance(registered, dict): command_names.update(self._valid_name_list(registered.get("generic"))) skill_names = ( - {self._skill_name_for_command(command["name"]) for command in manifest.commands} - if manifest is not None and skills_mode_active else set() + { + self._skill_name_for_command(command["name"]) + for command in manifest.commands + } + if manifest is not None and skills_mode_active + else set() ) skill_names.update(self._valid_name_list(metadata.get("registered_skills"))) paths: set[Path] = set() @@ -2029,12 +2067,15 @@ def _snapshot_generic_refresh_artifacts( relative = path.relative_to(root).as_posix() if hashes.get(relative) == hashlib.sha256(content).hexdigest(): snapshot[path] = ( - content, os.readlink(path) if path.is_symlink() else None, True + content, + os.readlink(path) if path.is_symlink() else None, + True, ) return snapshot def _restore_generic_refresh_artifacts( - self, snapshot: Dict[Path, tuple[bytes | None, str | None, bool]], + self, + snapshot: Dict[Path, tuple[bytes | None, str | None, bool]], extension_id: str, ) -> None: """Restore prior owned files and remove only outputs absent before refresh.""" @@ -2079,11 +2120,17 @@ def _restore_generic_refresh_artifacts( except (OSError, ValueError) as exc: errors.append(f"{path}: {exc}") if errors: - raise ExtensionError("Could not restore generic artifacts: " + "; ".join(errors)) + raise ExtensionError( + "Could not restore generic artifacts: " + "; ".join(errors) + ) def _generic_owned_names( - self, metadata: Dict[str, Any], names: List[str], *, - skills: bool, extension_id: str, + self, + metadata: Dict[str, Any], + names: List[str], + *, + skills: bool, + extension_id: str, ) -> List[str]: """Keep customized or untracked generic artifacts out of cleanup.""" from ..integrations.generic import registration_directory @@ -2096,7 +2143,9 @@ def _generic_owned_names( root = self.project_root.resolve() owned = [] for name in names: - path = output_dir / name / "SKILL.md" if skills else output_dir / f"{name}.md" + path = ( + output_dir / name / "SKILL.md" if skills else output_dir / f"{name}.md" + ) try: _validate_safe_shared_directory(root, path.parent) except (OSError, ValueError): @@ -2129,7 +2178,8 @@ def _complete_generic_refresh( self._generic_owned_names( metadata, missing, skills=skills, extension_id=extension_id ) - if missing else [] + if missing + else [] ) absent = sorted(set(missing) - set(retained)) if absent: @@ -2146,9 +2196,11 @@ def _remove_generic_artifact_paths( manifest = self.get_extension(extension_id) registered = metadata.get("registered_commands", {}) - command_names = set( - self._valid_name_list(registered.get("generic", [])) - ) if isinstance(registered, dict) else set() + command_names = ( + set(self._valid_name_list(registered.get("generic", []))) + if isinstance(registered, dict) + else set() + ) skill_names = set(self._valid_name_list(metadata.get("registered_skills", []))) if manifest is not None: command_names.update( @@ -2171,9 +2223,7 @@ def _remove_generic_artifact_paths( name = Path(relative) if name.is_absolute() or ".." in name.parts: continue - skill_output = ( - name.name == "SKILL.md" and name.parent.name in skill_names - ) + skill_output = name.name == "SKILL.md" and name.parent.name in skill_names if skill_output and not skills: continue if not skill_output and name.name not in { @@ -2250,11 +2300,14 @@ def _unregister_extension_skills( if generic_hashes is not None and skill_file.is_relative_to( self.project_root.resolve() ): - relative = skill_file.relative_to(self.project_root.resolve()).as_posix() + relative = skill_file.relative_to( + self.project_root.resolve() + ).as_posix() if Path(relative).parent.parent in generic_roots: - if generic_hashes.get(relative) != hashlib.sha256( - skill_file.read_bytes() - ).hexdigest(): + if ( + generic_hashes.get(relative) + != hashlib.sha256(skill_file.read_bytes()).hexdigest() + ): continue skill_file.unlink() try: @@ -2410,6 +2463,164 @@ def install_from_directory( force: bool = False, *, catalog_name: str | None = None, + ) -> ExtensionManifest: + """Install an extension as a rollback-safe transaction.""" + if priority < 1: + raise ValidationError("Priority must be a positive integer (1 or higher)") + + manifest = ExtensionManifest(source_dir / "extension.yml") + self.check_compatibility(manifest, speckit_version) + if not force and self.registry.is_installed(manifest.id): + raise ExtensionError( + f"Extension '{manifest.id}' is already installed. " + f"Use 'specify extension remove {manifest.id}' first, " + f"or retry with --force to overwrite." + ) + self._validate_install_conflicts(manifest) + dest_dir = self.extensions_dir / manifest.id + try: + same_location = source_dir.resolve(strict=False) == dest_dir.resolve( + strict=False + ) + except (OSError, RuntimeError): + same_location = source_dir.absolute() == dest_dir.absolute() + if same_location: + raise ValidationError( + f"Source path is the install destination for '{manifest.id}' " + f"({dest_dir}). Refusing to proceed to avoid deleting the " + "extension. Install from a copy in a different location instead." + ) + + was_installed = self.registry.is_installed(manifest.id) + if dest_dir.exists() and not was_installed: + # User-preserved config recovery uses the established installer path. + self._install_transaction_artifacts = {"commands": {}, "skills": []} + try: + return self._install_from_directory_unchecked( + source_dir, + speckit_version, + register_commands=register_commands, + priority=priority, + link_commands=link_commands, + force=force, + catalog_name=catalog_name, + ) + finally: + del self._install_transaction_artifacts + old_metadata = self.registry.get(manifest.id) if was_installed else None + old_registry_data = copy.deepcopy(self.registry.data) + registry_path = self.registry.registry_path + old_registry_bytes = ( + registry_path.read_bytes() if registry_path.is_file() else None + ) + hook_path = self.project_root / ".specify" / "extensions.yml" + old_hook_bytes = hook_path.read_bytes() if hook_path.is_file() else None + self.extensions_dir.mkdir(parents=True, exist_ok=True) + backup_root = Path( + tempfile.mkdtemp(prefix=f".install-{manifest.id}-", dir=self.extensions_dir) + ) + backup_dir = backup_root / "extension" + transaction_artifacts: dict[str, Any] = {"commands": {}, "skills": []} + previous_transaction_artifacts = getattr( + self, "_install_transaction_artifacts", None + ) + self._install_transaction_artifacts = transaction_artifacts + try: + if dest_dir.is_dir() and was_installed: + shutil.copytree(dest_dir, backup_dir, symlinks=True) + return self._install_from_directory_unchecked( + source_dir, + speckit_version, + register_commands=register_commands, + priority=priority, + link_commands=link_commands, + force=force, + catalog_name=catalog_name, + ) + except BaseException: + # Remove artifacts even if registry.add never committed: remove() + # only knows about previously committed ownership metadata. + try: + if transaction_artifacts["commands"]: + CommandRegistrar().unregister_commands( + transaction_artifacts["commands"], self.project_root + ) + self._unregister_extension_skills( + transaction_artifacts["skills"], manifest.id + ) + HookExecutor(self.project_root).unregister_hooks(manifest.id) + except BaseException: + pass + # Generic integration rollback deliberately converts a failed install + # into a config-only, unregistered extension directory. Preserve that + # user-facing recovery state instead of restoring the pre-install + # registered snapshot captured by this outer transaction. + preserved_generic_config = ( + dest_dir.is_dir() + and self._has_keep_config_marker(dest_dir) + and not (dest_dir / "extension.yml").exists() + and all( + child.name == ".keep-config" + or child.name.endswith(("-config.yml", "-config.local.yml")) + for child in dest_dir.iterdir() + ) + ) + if preserved_generic_config: + raise + + # Remove any partial install, then restore the previous on-disk + # extension and metadata/config snapshots byte-for-byte. + try: + if self.registry.is_installed(manifest.id): + self.remove(manifest.id) + except BaseException: + pass + if dest_dir.is_symlink(): + dest_dir.unlink() + elif dest_dir.exists(): + shutil.rmtree(dest_dir) + if backup_dir.is_dir(): + # Restore either a registered prior install or an unregistered + # `--keep-config` directory captured before validation/removal. + shutil.copytree(backup_dir, dest_dir, symlinks=True) + self.registry.data = old_registry_data + if old_registry_bytes is None: + registry_path.unlink(missing_ok=True) + else: + registry_path.parent.mkdir(parents=True, exist_ok=True) + registry_path.write_bytes(old_registry_bytes) + if old_hook_bytes is None: + hook_path.unlink(missing_ok=True) + else: + hook_path.parent.mkdir(parents=True, exist_ok=True) + hook_path.write_bytes(old_hook_bytes) + if was_installed and old_metadata: + # Recreate owned agent artifacts from the restored manifest; + # registration is idempotent and scoped to the extension. + try: + restored = ExtensionManifest(dest_dir / "extension.yml") + self._register_commands_for_active_agent(restored, dest_dir) + self._register_extension_skills(restored, dest_dir, force=True) + except BaseException: + pass + raise + finally: + if previous_transaction_artifacts is None: + del self._install_transaction_artifacts + else: + self._install_transaction_artifacts = previous_transaction_artifacts + shutil.rmtree(backup_root, ignore_errors=True) + + def _install_from_directory_unchecked( + self, + source_dir: Path, + speckit_version: str, + register_commands: bool = True, + priority: int = 10, + link_commands: bool = False, + force: bool = False, + *, + catalog_name: str | None = None, ) -> ExtensionManifest: """Install extension from a local directory. @@ -2456,7 +2667,9 @@ def install_from_directory( from .. import load_init_options active_options = load_init_options(self.project_root) - generic_active = isinstance(active_options, dict) and active_options.get("ai") == "generic" + generic_active = ( + isinstance(active_options, dict) and active_options.get("ai") == "generic" + ) if register_commands and manifest.commands: state, state_error = try_read_integration_json(self.project_root) if state_error is not None and generic_active: @@ -2470,17 +2683,22 @@ def install_from_directory( "Cannot register extension commands: cannot read integration settings: " f"{detail}" ) - generic_default = default_integration_key(state) == "generic" if state else False + generic_default = ( + default_integration_key(state) == "generic" if state else False + ) if state is not None and (generic_default or generic_active): if generic_default != generic_active: raise ExtensionError( "Cannot register generic extension commands: generic integration " "and init options disagree" ) - parsed_options = integration_setting(state, "generic").get("parsed_options") + parsed_options = integration_setting(state, "generic").get( + "parsed_options" + ) configured_skills = ( parsed_options.get("skills", False) - if isinstance(parsed_options, dict) else False + if isinstance(parsed_options, dict) + else False ) init_skills = active_options.get("ai_skills", False) if ( @@ -2498,11 +2716,16 @@ def install_from_directory( try: output_dir = registration_directory(self.project_root) except (OSError, ValueError) as exc: - raise ExtensionError(f"Cannot register generic extension commands: {exc}") from exc + raise ExtensionError( + f"Cannot register generic extension commands: {exc}" + ) from exc source_root = source_dir.resolve() for command in manifest.commands: source_file = (source_root / command["file"]).resolve() - if not source_file.is_relative_to(source_root) or not source_file.is_file(): + if ( + not source_file.is_relative_to(source_root) + or not source_file.is_file() + ): raise ExtensionError( "Cannot register generic extension commands: missing source " f"'{command['file']}'" @@ -2516,22 +2739,32 @@ def install_from_directory( ) from exc skills = is_ai_skills_enabled(active_options) names = ( - {self._skill_name_for_command(command["name"]) for command in manifest.commands} - if skills else self._collect_manifest_command_names(manifest) + { + self._skill_name_for_command(command["name"]) + for command in manifest.commands + } + if skills + else self._collect_manifest_command_names(manifest) ) owned = ( - set(self._generic_owned_names( - self.registry.get(manifest.id) or {}, list(names), - skills=skills, extension_id=manifest.id, - )) - if force and self.registry.is_installed(manifest.id) else set() + set( + self._generic_owned_names( + self.registry.get(manifest.id) or {}, + list(names), + skills=skills, + extension_id=manifest.id, + ) + ) + if force and self.registry.is_installed(manifest.id) + else set() ) for name in sorted(names): target = output_dir / name if skills else output_dir / f"{name}.md" if not (target.exists() or target.is_symlink()): continue if name in owned and ( - not skills or not any(child.name != "SKILL.md" for child in target.iterdir()) + not skills + or not any(child.name != "SKILL.md" for child in target.iterdir()) ): continue raise ExtensionError( @@ -2635,9 +2868,7 @@ def _recognized_config_names( if not directory.is_dir(): return names for entry in directory.iterdir(): - if not entry.name.endswith( - ("-config.yml", "-config.local.yml") - ): + if not entry.name.endswith(("-config.yml", "-config.local.yml")): continue if follow_symlinks: if entry.is_file() and not entry.is_symlink(): @@ -2652,9 +2883,7 @@ def _recognized_config_names( conflicting: set[str] = set() staged_names = _recognized_config_names(rescue_staging_dir) - live_names = _recognized_config_names( - dest_dir, follow_symlinks=False - ) + live_names = _recognized_config_names(dest_dir, follow_symlinks=False) def _matches_source_config_baseline(config_name: str) -> bool: source_file = source_dir / config_name @@ -2682,9 +2911,7 @@ def _matches_source_config_baseline(config_name: str) -> bool: # Only truly divergent live-only configs are conflicts. live_only = live_names - staged_names conflicting.update( - name - for name in live_only - if not _matches_source_config_baseline(name) + name for name in live_only if not _matches_source_config_baseline(name) ) # Load original permission bits from the sidecar JSON written by # the staging step. Staged files are kept at mode 0o600 so that @@ -2758,9 +2985,10 @@ def _matches_source_config_baseline(config_name: str) -> bool: except OSError: conflicting.add(staged_name) else: - if live_bytes != staged_bytes or stat.S_IMODE( - live_stat.st_mode - ) != staged_mode: + if ( + live_bytes != staged_bytes + or stat.S_IMODE(live_stat.st_mode) != staged_mode + ): conflicting.add(staged_name) stranded_configs[staged_name] = (staged_bytes, staged_mode) if conflicting: @@ -2800,9 +3028,8 @@ def _matches_source_config_baseline(config_name: str) -> bool: or self._is_legacy_keep_config_leftover(dest_dir) ) ): - for cfg_file in ( - list(dest_dir.glob("*-config.yml")) - + list(dest_dir.glob("*-config.local.yml")) + for cfg_file in list(dest_dir.glob("*-config.yml")) + list( + dest_dir.glob("*-config.local.yml") ): if cfg_file.is_symlink(): # `remove --keep-config` preserves a symlinked config @@ -2866,7 +3093,10 @@ def _matches_source_config_baseline(config_name: str) -> bool: # bytes' "\n" into "\r\n" as they are written. fd = os.open( str(staged), - os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0), + os.O_WRONLY + | os.O_CREAT + | os.O_EXCL + | getattr(os, "O_BINARY", 0), 0o600, ) try: @@ -3020,7 +3250,11 @@ def rollback_generic_registration() -> None: root = self.project_root.resolve() installed_root = dest_dir.resolve() for name in names: - path = output_dir / name / "SKILL.md" if skills else output_dir / f"{name}.md" + path = ( + output_dir / name / "SKILL.md" + if skills + else output_dir / f"{name}.md" + ) try: _validate_safe_shared_directory(root, path.parent) except (OSError, ValueError): @@ -3050,7 +3284,9 @@ def rollback_generic_registration() -> None: if ( config_file.is_file() and not config_file.is_symlink() - and config_file.name.endswith(("-config.yml", "-config.local.yml")) + and config_file.name.endswith( + ("-config.yml", "-config.local.yml") + ) ): shutil.copy2(config_file, dest_dir / config_file.name) preserved.add(config_file.name) @@ -3081,10 +3317,15 @@ def rollback_generic_registration() -> None: registered_skills = self._register_extension_skills( manifest, dest_dir, link_outputs=link_commands ) + self._install_transaction_artifacts["commands"] = registered_commands + self._install_transaction_artifacts["skills"] = registered_skills + if register_commands and generic_active and manifest.commands: expected = set(names) actual = set( - registered_skills if skills else registered_commands.get("generic", []) + registered_skills + if skills + else registered_commands.get("generic", []) ) missing = expected - actual if missing: @@ -3094,7 +3335,8 @@ def rollback_generic_registration() -> None: ) generic_hashes = ( self._generic_artifact_hashes(registered_commands, registered_skills) - if generic_active else {} + if generic_active + else {} ) # Register hooks and update installed list in extensions.yml @@ -3181,8 +3423,11 @@ def rollback_generic_registration() -> None: from .. import _print_cli_warning _print_cli_warning( - "remove", "configuration backup", str(backup_config_dir), - exc, continuing="The extension was installed; the backup remains.", + "remove", + "configuration backup", + str(backup_config_dir), + exc, + continuing="The extension was installed; the backup remains.", ) # Post-commit cleanup: the registry now records this extension as @@ -3219,6 +3464,7 @@ def rollback_generic_registration() -> None: # did, and it would cost a per-caller scan-scope argument on an otherwise simple, # widely-used interface. The simpler call wins. from .. import ensure_executable_scripts + ensure_executable_scripts(self.project_root) return manifest @@ -3353,7 +3599,9 @@ def _target_follows_preserved_convention(target_name: str) -> bool: "-config.local.yml" ) - def scaffold_config(self, extension_id: str) -> tuple[List[str], List[str], List[str]]: + def scaffold_config( + self, extension_id: str + ) -> tuple[List[str], List[str], List[str]]: """Deploy config templates from an installed extension to the project. Reads the extension's manifest provides.config section and copies @@ -3379,12 +3627,9 @@ def scaffold_config(self, extension_id: str) -> tuple[List[str], List[str], List provides = manifest.data.get("provides", {}) raw_config = provides.get("config", []) - config_is_malformed = ( - "config" in provides - and ( - not isinstance(raw_config, list) - or not all(isinstance(entry, dict) for entry in raw_config) - ) + config_is_malformed = "config" in provides and ( + not isinstance(raw_config, list) + or not all(isinstance(entry, dict) for entry in raw_config) ) if config_is_malformed: return deployed, skipped_existing, ["provides.config"] @@ -3408,7 +3653,11 @@ def scaffold_config(self, extension_id: str) -> tuple[List[str], List[str], List for config_entry in manifest.config: template_name = config_entry.get("template", "") target_name = config_entry.get("name", template_name) - failure_name = target_name if isinstance(target_name, str) and target_name else "provides.config" + failure_name = ( + target_name + if isinstance(target_name, str) and target_name + else "provides.config" + ) if not isinstance(template_name, str) or not template_name: failed.append(failure_name) continue @@ -3518,9 +3767,7 @@ def remove(self, extension_id: str, keep_config: bool = False) -> bool: if "generic" in safe_commands: safe_commands.pop("generic") if safe_commands: - CommandRegistrar().unregister_commands( - safe_commands, self.project_root - ) + CommandRegistrar().unregister_commands(safe_commands, self.project_root) if metadata: self._remove_generic_artifact_paths(extension_id, metadata) @@ -3528,7 +3775,9 @@ def remove(self, extension_id: str, keep_config: bool = False) -> bool: self._unregister_extension_skills( registered_skills, extension_id, - generic_hashes=metadata.get("generic_artifact_hashes") if metadata else None, + generic_hashes=metadata.get("generic_artifact_hashes") + if metadata + else None, ) if keep_config: @@ -3587,7 +3836,11 @@ def disable_generic_extension_artifacts(self, extension_id: str) -> None: raise ExtensionError(f"Extension '{extension_id}' is not installed") registered = metadata.get("registered_commands", {}) - commands = self._valid_name_list(registered.get("generic")) if isinstance(registered, dict) else [] + commands = ( + self._valid_name_list(registered.get("generic")) + if isinstance(registered, dict) + else [] + ) skills = self._valid_name_list(metadata.get("registered_skills", [])) hashes = metadata.get("generic_artifact_hashes", {}) has_artifacts = bool(commands or skills or hashes) @@ -3605,7 +3858,12 @@ def disable_generic_extension_artifacts(self, extension_id: str) -> None: if name.is_absolute() or ".." in name.parts: continue path = self.project_root.resolve() / name - if path.parent.resolve().is_relative_to(self.project_root.resolve()) and path.is_file(): + if ( + path.parent.resolve().is_relative_to( + self.project_root.resolve() + ) + and path.is_file() + ): if path.is_symlink() and not path.resolve().is_relative_to( (self.extensions_dir / extension_id).resolve() ): @@ -3621,14 +3879,20 @@ def disable_generic_extension_artifacts(self, extension_id: str) -> None: if not hashes: for names, is_skill in ((commands, False), (skills, True)): for name in names: - path = directory / name / "SKILL.md" if is_skill else directory / f"{name}.md" + path = ( + directory / name / "SKILL.md" + if is_skill + else directory / f"{name}.md" + ) if path.exists() or path.is_symlink(): raise ExtensionError( f"Cannot disable '{extension_id}': generic artifact {path} " "was modified or is not owned; preserve it and remove it manually" ) snapshot = self._snapshot_generic_refresh_artifacts( - extension_id, self.get_extension(extension_id), metadata, + extension_id, + self.get_extension(extension_id), + metadata, skills_mode_active=bool(skills), ) @@ -3638,16 +3902,22 @@ def disable_generic_extension_artifacts(self, extension_id: str) -> None: self._remove_generic_artifact_paths(extension_id, metadata) if skills: self._unregister_extension_skills( - skills, extension_id, skills_dir=directory, + skills, + extension_id, + skills_dir=directory, generic_hashes=metadata.get("generic_artifact_hashes", {}), ) new_commands = dict(registered) if isinstance(registered, dict) else {} new_commands.pop("generic", None) - updates.update({ - "registered_commands": new_commands, - "registered_skills": self._extension_owned_skill_names(skills, extension_id), - "generic_artifact_hashes": {}, - }) + updates.update( + { + "registered_commands": new_commands, + "registered_skills": self._extension_owned_skill_names( + skills, extension_id + ), + "generic_artifact_hashes": {}, + } + ) registry_update_started = True self.registry.update(extension_id, updates) except Exception as exc: @@ -3681,6 +3951,7 @@ def unregister_agent_artifacts( *, enabled_only: bool = False, commands_only: bool = False, + extension_ids: Optional[set[str]] = None, ) -> None: """Remove extension files registered for a specific agent. @@ -3715,6 +3986,8 @@ def unregister_agent_artifacts( agent_skills_dir = resolve_skills_dir(self.project_root, agent_name) for ext_id, metadata in self.registry.list().items(): + if extension_ids is not None and ext_id not in extension_ids: + continue if enabled_only and not metadata.get("enabled", True): continue @@ -3764,10 +4037,13 @@ def unregister_agent_artifacts( # to clean up; the fast path below is a safe no-op in that # case (every candidate skill_subdir.is_dir() check fails). self._unregister_extension_skills( - registered_skills, ext_id, skills_dir=agent_skills_dir, + registered_skills, + ext_id, + skills_dir=agent_skills_dir, generic_hashes=( metadata.get("generic_artifact_hashes") - if agent_name == "generic" else None + if agent_name == "generic" + else None ), ) @@ -3819,9 +4095,7 @@ def _retire_legacy_flat_extension_commands( integration = get_integration(agent_name) legacy_dir = getattr(integration, "legacy_flat_command_dir", None) - legacy_extension = getattr( - integration, "legacy_flat_command_extension", None - ) + legacy_extension = getattr(integration, "legacy_flat_command_extension", None) if ( not isinstance(legacy_dir, str) or not legacy_dir @@ -3878,7 +4152,9 @@ def safe_project_dir(relative: str) -> Optional[Path]: return removed - def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool = False) -> None: + def register_enabled_extensions_for_agent( + self, agent_name: str, *, force: bool = False + ) -> None: """Register installed, enabled extensions for ``agent_name``. Command-file registration is scoped to the explicit ``agent_name`` @@ -3941,7 +4217,9 @@ def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool try: if agent_name == "generic": generic_snapshot = self._snapshot_generic_refresh_artifacts( - ext_id, manifest, metadata, + ext_id, + manifest, + metadata, skills_mode_active=skills_mode_active, ) updates: Dict[str, Any] = {} @@ -3991,9 +4269,9 @@ def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool # actual removal is deferred until after the skills # phase below completes without raising (#2948). registered_commands = metadata.get("registered_commands", {}) - if isinstance(registered_commands, dict) and registered_commands.get( - agent_name - ): + if isinstance( + registered_commands, dict + ) and registered_commands.get(agent_name): deferred_stale_commands = self._valid_name_list( registered_commands.get(agent_name) ) @@ -4084,19 +4362,26 @@ def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool for cmd_name in (registered or []) } to_remove = [ - name for name in owned_here + name + for name in owned_here if name in replaced_skill_names ] if agent_name == "generic": to_remove = self._generic_owned_names( - metadata, to_remove, skills=True, extension_id=ext_id + metadata, + to_remove, + skills=True, + extension_id=ext_id, ) if to_remove: self._unregister_extension_skills( - to_remove, ext_id, skills_dir=agent_skills_dir, + to_remove, + ext_id, + skills_dir=agent_skills_dir, generic_hashes=( metadata.get("generic_artifact_hashes") - if agent_name == "generic" else None + if agent_name == "generic" + else None ), ) # registered_skills is a single flat list @@ -4169,7 +4454,8 @@ def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool ) fully_replaced = [ - cmd_name for cmd_name in deferred_stale_commands + cmd_name + for cmd_name in deferred_stale_commands if group_fully_replaced.get( alias_to_primary.get(cmd_name, cmd_name), False ) @@ -4177,7 +4463,9 @@ def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool if fully_replaced: if agent_name == "generic": fully_replaced = self._generic_owned_names( - metadata, fully_replaced, skills=False, + metadata, + fully_replaced, + skills=False, extension_id=ext_id, ) registrar.unregister_commands( @@ -4191,7 +4479,8 @@ def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool ): new_registered = copy.deepcopy(registered_commands) remaining_commands = [ - c for c in new_registered[agent_name] + c + for c in new_registered[agent_name] if c not in fully_replaced ] if remaining_commands: @@ -4279,7 +4568,9 @@ def list_installed(self) -> List[Dict[str, Any]]: "installed_at": metadata.get("installed_at"), "command_count": len(manifest.commands), "hook_count": hook_count, - "_json_author": author if isinstance(author, str) and author else None, + "_json_author": author + if isinstance(author, str) and author + else None, "_json_source": metadata.get("source"), "_json_provides": { "commands": len(manifest.commands), @@ -4304,7 +4595,12 @@ def list_installed(self) -> List[Dict[str, Any]]: "hook_count": 0, "_json_author": None, "_json_source": metadata.get("source"), - "_json_provides": {"commands": 0, "templates": 0, "scripts": 0, "hooks": 0}, + "_json_provides": { + "commands": 0, + "templates": 0, + "scripts": 0, + "hooks": 0, + }, } ) @@ -4520,7 +4816,9 @@ def _resolve_github_release_asset_api_url( passing the ``github`` provider hosts from ``auth.json`` so GitHub Enterprise Server release assets resolve via ``/api/v3``. """ - from specify_cli.authentication.github_http import resolve_github_release_asset_api_url + from specify_cli.authentication.github_http import ( + resolve_github_release_asset_api_url, + ) from specify_cli.authentication.http import github_provider_hosts return resolve_github_release_asset_api_url( @@ -5141,9 +5439,7 @@ def download_extension_info( if not download_url: raise ExtensionError(f"Extension '{extension_id}' has no download URL") if not isinstance(download_url, str): - raise ExtensionError( - f"Extension download URL is malformed: {download_url}" - ) + raise ExtensionError(f"Extension download URL is malformed: {download_url}") # Validate download URL requires HTTPS (prevent man-in-the-middle attacks) from urllib.parse import urlparse @@ -5163,9 +5459,7 @@ def download_extension_info( f"Extension download URL is malformed: {download_url}" ) from None if not hostname: - raise ExtensionError( - f"Extension download URL is malformed: {download_url}" - ) + raise ExtensionError(f"Extension download URL is malformed: {download_url}") if not is_https_or_localhost_http(download_url): raise ExtensionError( f"Extension download URL must use HTTPS: {download_url}" @@ -5205,9 +5499,7 @@ def download_extension_info( label=f"extension '{extension_id}' download", ) final_url = ( - response.geturl() - if hasattr(response, "geturl") - else download_url + response.geturl() if hasattr(response, "geturl") else download_url ) content_type = ( response.getheader("Content-Type") diff --git a/src/specify_cli/extensions/_commands.py b/src/specify_cli/extensions/_commands.py index ce6d229878..c8a93c739b 100644 --- a/src/specify_cli/extensions/_commands.py +++ b/src/specify_cli/extensions/_commands.py @@ -5,6 +5,7 @@ re-fetch package helpers at call time so existing monkeypatch paths keep working. Cohesive private phases use ``_command__*.py`` modules. """ + from __future__ import annotations import errno @@ -34,25 +35,30 @@ add_completion=False, ) + # Root helpers re-fetched at call time so test monkeypatching of # `specify_cli.` keeps working after the move. def _require_specify_project(*args, **kwargs): from .. import _require_specify_project as _f + return _f(*args, **kwargs) def _locate_bundled_extension(*args, **kwargs): from .. import _locate_bundled_extension as _f + return _f(*args, **kwargs) def load_init_options(*args, **kwargs): from .. import load_init_options as _f + return _f(*args, **kwargs) def _display_project_path(*args, **kwargs): from .. import _display_project_path as _f + return _f(*args, **kwargs) @@ -97,6 +103,34 @@ def _archive_extension_directory(*args, **kwargs): return _helper(*args, **kwargs) +def _refresh_presets_and_warn(project_root: Path) -> None: + """Re-register enabled preset overrides after extension stack changes. + + Preset regex selectors expand against currently available lower layers, so + installing, removing, enabling, disabling, or reprioritizing an extension + can change which concrete command declarations are materialized. Keep the + normal preset enablement and active-integration rules by using its existing + integration-switch registration path. + """ + try: + from .._init_options import load_init_options + from ..presets import PresetManager + + agent = load_init_options(project_root).get("ai") + if agent: + PresetManager(project_root).register_enabled_presets_for_agent(agent) + except Exception as exc: + from .. import _print_cli_warning + + _print_cli_warning( + "reconcile preset artifacts after extension change", + "project", + str(project_root), + exc, + continuing="Continuing; run 'specify integration use' to retry registration.", + ) + + def _refresh_events_and_warn(project_root: Path) -> None: """Refresh native event config and surface failures (R3). @@ -146,9 +180,7 @@ def install_extension_from_url( from . import ExtensionCatalog, ExtensionError if not is_https_or_localhost_http(url): - raise ExtensionError( - "URL must use HTTPS (HTTP is only allowed for localhost)" - ) + raise ExtensionError("URL must use HTTPS (HTTP is only allowed for localhost)") download_dir = _validate_safe_cache_dir(project_root) archive_filename = f"extension-url-download-{uuid4().hex}.archive" @@ -209,9 +241,7 @@ def install_extension_from_url( ) from exc format_source = ( - final_url - if archive_format_from_name(final_url) is not None - else url + final_url if archive_format_from_name(final_url) is not None else url ) try: detect_archive_format( @@ -282,7 +312,9 @@ def _resolve_installed_extension( return (ext["id"], ext["name"]) # If not found by ID, try display name match - name_matches = [ext for ext in installed_extensions if ext["name"].lower() == argument.lower()] + name_matches = [ + ext for ext in installed_extensions if ext["name"].lower() == argument.lower() + ] if len(name_matches) == 1: # Unique display-name match @@ -311,7 +343,9 @@ def _resolve_installed_extension( # No match by ID or display name if allow_not_found: return (None, None) - console.print(f"[red]Error:[/red] Extension '{_escape_markup(argument)}' is not installed") + console.print( + f"[red]Error:[/red] Extension '{_escape_markup(argument)}' is not installed" + ) raise typer.Exit(1) @@ -375,7 +409,9 @@ def _resolve_catalog_extension( ) console.print(table) console.print("\nPlease rerun using the extension ID:") - console.print(f" [bold]specify extension {command_name} [/bold]") + console.print( + f" [bold]specify extension {command_name} [/bold]" + ) raise typer.Exit(1) # Not found @@ -432,9 +468,7 @@ def _verify_leaf_identity(fd: int, path: Path) -> None: or path_stat.st_dev != open_stat.st_dev or path_stat.st_ino != open_stat.st_ino ): - raise OSError( - errno.ENOTDIR, "Download file changed between creation and open" - ) + raise OSError(errno.ENOTDIR, "Download file changed between creation and open") def _validate_safe_cache_dir(project_root: Path) -> Path: @@ -568,9 +602,7 @@ def _safe_open_download_zip( classes on every supported platform. """ if _has_secure_dir_fd(): - return _open_download_zip_via_dir_fd( - project_root, download_dir, zip_filename - ) + return _open_download_zip_via_dir_fd(project_root, download_dir, zip_filename) return _open_download_zip_via_paths(project_root, download_dir, zip_filename) @@ -623,15 +655,15 @@ def _open_download_zip_via_paths( project_root_resolved = project_root.resolve() if download_dir.is_symlink() or not download_dir.is_dir(): - raise OSError( - errno.ENOTDIR, "Download cache directory is not a real directory" - ) + raise OSError(errno.ENOTDIR, "Download cache directory is not a real directory") try: download_dir.resolve().relative_to(project_root_resolved) except (OSError, ValueError): raise OSError(errno.ENOTDIR, "Download cache directory escapes project root") if zip_path.is_symlink(): - raise OSError(errno.ELOOP, "Refusing to write through a symlinked download file") + raise OSError( + errno.ELOOP, "Refusing to write through a symlinked download file" + ) flags = os.O_RDWR | os.O_CREAT | os.O_EXCL flags |= getattr(os, "O_NOFOLLOW", 0) diff --git a/src/specify_cli/extensions/command_add.py b/src/specify_cli/extensions/command_add.py index 30a4ff717c..966f292395 100644 --- a/src/specify_cli/extensions/command_add.py +++ b/src/specify_cli/extensions/command_add.py @@ -3,6 +3,7 @@ Registered by ``_commands.register()``; shared command infrastructure lives in ``_commands.py``. """ + from __future__ import annotations from pathlib import Path @@ -21,13 +22,28 @@ 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"), + from_url: Optional[str] = typer.Option( + None, "--from", help="Install from custom URL" + ), force: bool = typer.Option(False, "--force", help="Overwrite if already installed"), - priority: int = typer.Option(10, "--priority", help="Resolution priority (lower = higher precedence, default 10)"), - version: Optional[str] = typer.Option(None, "--version", help="Install an exact version from a catalog"), + priority: int = typer.Option( + 10, + "--priority", + help="Resolution priority (lower = higher precedence, default 10)", + ), + version: Optional[str] = typer.Option( + None, "--version", help="Install an exact version from a catalog" + ), ): """Install an extension.""" - from . import ExtensionManager, ExtensionCatalog, ExtensionError, ValidationError, CompatibilityError, REINSTALL_COMMAND + from . import ( + ExtensionManager, + ExtensionCatalog, + ExtensionError, + ValidationError, + CompatibilityError, + REINSTALL_COMMAND, + ) # Compatibility callers invoke this function directly, in which case # Typer supplies its OptionInfo object instead of a parsed option value. @@ -37,10 +53,14 @@ def extension_add( project_root = _commands._require_specify_project() # Validate priority if priority < 1: - console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") + console.print( + "[red]Error:[/red] Priority must be a positive integer (1 or higher)" + ) raise typer.Exit(1) if version is not None and (not version.strip() or dev or from_url): - console.print("[red]Error:[/red] --version requires a catalog install (without --dev or --from).") + console.print( + "[red]Error:[/red] --version requires a catalog install (without --dev or --from)." + ) raise typer.Exit(1) manager = ExtensionManager(project_root) @@ -84,15 +104,17 @@ def extension_add( # Warn about untrusted sources — default-deny confirmation console.print() - console.print(Panel( - f"[bold]You are installing an extension directly from an external URL,\n" - f"bypassing your trusted (install-allowed) extension catalogs.[/bold]\n\n" - f"URL: {safe_url}\n\n" - f"Only install extensions from sources you trust.", - title="[bold yellow]⚠ Untrusted Source[/bold yellow]", - border_style="yellow", - padding=(1, 2), - )) + console.print( + Panel( + f"[bold]You are installing an extension directly from an external URL,\n" + f"bypassing your trusted (install-allowed) extension catalogs.[/bold]\n\n" + f"URL: {safe_url}\n\n" + f"Only install extensions from sources you trust.", + title="[bold yellow]⚠ Untrusted Source[/bold yellow]", + border_style="yellow", + padding=(1, 2), + ) + ) console.print() confirm = typer.confirm("Continue with installation?", default=False) if not confirm: @@ -108,22 +130,28 @@ def extension_add( source_path = Path(extension).expanduser().resolve() safe_source_path = _escape_markup(str(source_path)) if not source_path.exists(): - console.print(f"[red]Error:[/red] Directory not found: {safe_source_path}") + console.print( + f"[red]Error:[/red] Directory not found: {safe_source_path}" + ) raise typer.Exit(1) if not (source_path / "extension.yml").exists(): - console.print(f"[red]Error:[/red] No extension.yml found in {safe_source_path}") + console.print( + f"[red]Error:[/red] No extension.yml found in {safe_source_path}" + ) raise typer.Exit(1) if force: - console.print(f"[yellow]--force:[/yellow] Installing from [cyan]{safe_source_path}[/cyan] (will overwrite if already installed)...") + console.print( + f"[yellow]--force:[/yellow] Installing from [cyan]{safe_source_path}[/cyan] (will overwrite if already installed)..." + ) manifest = manager.install_from_directory( source_path, speckit_version, priority=priority, link_commands=True, - force=force + force=force, ) elif from_url: @@ -145,7 +173,8 @@ def extension_add( # Try bundled extensions first (shipped with spec-kit) bundled_path = ( _commands._locate_bundled_extension(extension) - if version is None else None + if version is None + else None ) if bundled_path is not None: manifest = manager.install_from_directory( @@ -160,10 +189,14 @@ def extension_add( extension, catalog, "add" ) if catalog_error: - console.print(f"[red]Error:[/red] Could not query extension catalog: {_escape_markup(str(catalog_error))}") + console.print( + f"[red]Error:[/red] Could not query extension catalog: {_escape_markup(str(catalog_error))}" + ) raise typer.Exit(1) if not ext_info: - console.print(f"[red]Error:[/red] Extension '{safe_extension}' not found in catalog") + console.print( + f"[red]Error:[/red] Extension '{safe_extension}' not found in catalog" + ) console.print("\nSearch available extensions:") console.print(" specify extension search") raise typer.Exit(1) @@ -190,24 +223,32 @@ def extension_add( ) raise typer.Exit(1) - # If catalog resolved a display name to an ID, check bundled again - resolved_id = ext_info['id'] - if version is not None and ext_info.get("bundled") and not ext_info.get("download_url"): + resolved_id = ext_info["id"] + if ( + version is not None + and ext_info.get("bundled") + and not ext_info.get("download_url") + ): from . import ExtensionManifest candidate = _commands._locate_bundled_extension(resolved_id) if candidate is not None: - bundled_manifest = ExtensionManifest(candidate / "extension.yml") + bundled_manifest = ExtensionManifest( + candidate / "extension.yml" + ) try: - packaged_matches = Version(bundled_manifest.version) == Version( - version - ) + packaged_matches = Version( + bundled_manifest.version + ) == Version(version) except InvalidVersion: packaged_matches = False if packaged_matches: bundled_path = candidate manifest = manager.install_from_directory( - bundled_path, speckit_version, priority=priority, force=force + bundled_path, + speckit_version, + priority=priority, + force=force, ) if bundled_path is None: console.print( @@ -220,7 +261,10 @@ def extension_add( bundled_path = _commands._locate_bundled_extension(resolved_id) if bundled_path is not None: manifest = manager.install_from_directory( - bundled_path, speckit_version, priority=priority, force=force + bundled_path, + speckit_version, + priority=priority, + force=force, ) if bundled_path is None: @@ -239,7 +283,9 @@ def extension_add( # Enforce install_allowed policy if not ext_info.get("_install_allowed", True): - catalog_name = _escape_markup(str(ext_info.get("_catalog_name", "community"))) + catalog_name = _escape_markup( + str(ext_info.get("_catalog_name", "community")) + ) resolved_id = _commands._command_safe_id(ext_info["id"]) console.print( f"[red]Error:[/red] '{safe_extension}' was found in the " @@ -263,8 +309,10 @@ def extension_add( raise typer.Exit(1) # Download extension archive (use the resolved catalog ID). - extension_id = ext_info['id'] - console.print(f"Downloading {_escape_markup(str(ext_info['name']))} v{_escape_markup(str(ext_info.get('version', 'unknown')))}...") + extension_id = ext_info["id"] + console.print( + f"Downloading {_escape_markup(str(ext_info['name']))} v{_escape_markup(str(ext_info.get('version', 'unknown')))}..." + ) archive_path = ( catalog.download_extension_info(ext_info) if version is not None @@ -279,23 +327,32 @@ def extension_add( force=force, catalog_name=ext_info.get("_catalog_name"), **( - {"expected_id": extension_id, "expected_version": version} - if version is not None else {} + { + "expected_id": extension_id, + "expected_version": version, + } + if version is not None + else {} ), ) finally: archive_path.unlink(missing_ok=True) console.print("\n[green]✓[/green] Extension installed successfully!") - console.print(f"\n[bold]{_escape_markup(str(manifest.name))}[/bold] (v{_escape_markup(str(manifest.version))})") + console.print( + f"\n[bold]{_escape_markup(str(manifest.name))}[/bold] (v{_escape_markup(str(manifest.version))})" + ) console.print(f" {_escape_markup(str(manifest.description))}") # #1: regenerate native event config for installed event-capable # integrations so the new extension's events take effect immediately. _commands._refresh_events_and_warn(project_root) + _commands._refresh_presets_and_warn(project_root) for warning in manifest.warnings: - console.print(f"\n[yellow]⚠ Compatibility warning:[/yellow] {_escape_markup(str(warning))}") + console.print( + f"\n[yellow]⚠ Compatibility warning:[/yellow] {_escape_markup(str(warning))}" + ) selected_ai = _commands.load_init_options(project_root).get("ai") is_cline = selected_ai == "cline" @@ -308,12 +365,14 @@ def extension_add( console.print("\n[bold cyan]Provided commands:[/bold cyan]") for cmd in manifest.commands: - cmd_name = cmd['name'] + cmd_name = cmd["name"] if is_cline: cmd_name = format_cline_command_name(cmd_name) elif is_forge: cmd_name = format_forge_command_name(cmd_name) - console.print(f" • {_escape_markup(str(cmd_name))} - {_escape_markup(str(cmd.get('description', '')))}") + console.print( + f" • {_escape_markup(str(cmd_name))} - {_escape_markup(str(cmd.get('description', '')))}" + ) # Report agent skills registration reg_meta = manager.registry.get(manifest.id) @@ -322,7 +381,9 @@ def extension_add( if not isinstance(reg_skills, list): reg_skills = [] if reg_skills: - console.print(f"\n[green]✓[/green] {len(reg_skills)} agent skill(s) auto-registered") + console.print( + f"\n[green]✓[/green] {len(reg_skills)} agent skill(s) auto-registered" + ) # Scaffold config templates automatically deployed, skipped, failed = manager.scaffold_config(manifest.id) @@ -332,7 +393,9 @@ def extension_add( for cfg in deployed: console.print(f" • {config_home}/{_escape_markup(str(cfg))}") if skipped: - console.print(f"\n[dim]Config files already exist (preserved): {_escape_markup(', '.join(skipped))}[/dim]") + console.print( + f"\n[dim]Config files already exist (preserved): {_escape_markup(', '.join(skipped))}[/dim]" + ) if failed: console.print( f"\n[yellow]Warning:[/yellow] Config templates not scaffolded: " diff --git a/src/specify_cli/extensions/command_disable.py b/src/specify_cli/extensions/command_disable.py index 16ef6683e1..32eb076036 100644 --- a/src/specify_cli/extensions/command_disable.py +++ b/src/specify_cli/extensions/command_disable.py @@ -3,6 +3,7 @@ Registered by ``_commands.register()``; shared command infrastructure lives in ``_commands.py``. """ + from __future__ import annotations import typer @@ -31,7 +32,7 @@ def extension_disable( # Update registry metadata = manager.registry.get(extension_id) - if metadata is None or not isinstance(metadata, dict): + if not extension_id or metadata is None or not isinstance(metadata, dict): console.print( f"[red]Error:[/red] Extension '{_escape_markup(str(extension_id))}' " "not found in registry (corrupted state)" @@ -39,12 +40,15 @@ def extension_disable( raise typer.Exit(1) if not metadata.get("enabled", True): - console.print(f"[yellow]Extension '{_escape_markup(str(display_name))}' is already disabled[/yellow]") + console.print( + f"[yellow]Extension '{_escape_markup(str(display_name))}' is already disabled[/yellow]" + ) raise typer.Exit(0) from .. import load_init_options - if load_init_options(project_root).get("ai") == "generic": + agent = load_init_options(project_root).get("ai") + if agent == "generic": from . import ExtensionError try: @@ -53,6 +57,10 @@ def extension_disable( console.print(f"[red]Error:[/red] {_escape_markup(str(exc))}") raise typer.Exit(1) from exc else: + # Remove this agent's tracked artifacts before flipping enabled. If + # cleanup fails, ownership metadata and enabled state remain retryable. + if agent: + manager.unregister_agent_artifacts(agent, extension_ids={extension_id}) manager.registry.update(extension_id, {"enabled": False}) # Disable hooks in extensions.yml @@ -64,10 +72,17 @@ def extension_disable( hook["enabled"] = False hook_executor.save_project_config(config) - console.print(f"[green]✓[/green] Extension '{_escape_markup(str(display_name))}' disabled") + console.print( + f"[green]✓[/green] Extension '{_escape_markup(str(display_name))}' disabled" + ) console.print("\nCommands will no longer be available. Hooks will not execute.") - console.print(f"To re-enable: specify extension enable {_escape_markup(str(extension_id))}") + console.print( + f"To re-enable: specify extension enable {_escape_markup(str(extension_id))}" + ) # #1: regenerate native event config so the disabled extension's events # are stripped from installed integrations. + # Extension mutations may change the expansion set for preset regex + # selectors; re-register enabled presets after refreshing native events. _commands._refresh_events_and_warn(project_root) + _commands._refresh_presets_and_warn(project_root) diff --git a/src/specify_cli/extensions/command_enable.py b/src/specify_cli/extensions/command_enable.py index 34bab2f279..ea88899ade 100644 --- a/src/specify_cli/extensions/command_enable.py +++ b/src/specify_cli/extensions/command_enable.py @@ -3,6 +3,7 @@ Registered by ``_commands.register()``; shared command infrastructure lives in ``_commands.py``. """ + from __future__ import annotations import typer @@ -31,7 +32,7 @@ def extension_enable( # Update registry metadata = manager.registry.get(extension_id) - if metadata is None or not isinstance(metadata, dict): + if not extension_id or metadata is None or not isinstance(metadata, dict): console.print( f"[red]Error:[/red] Extension '{_escape_markup(str(extension_id))}' " "not found in registry (corrupted state)" @@ -39,7 +40,9 @@ def extension_enable( raise typer.Exit(1) if metadata.get("enabled", True): - console.print(f"[yellow]Extension '{_escape_markup(str(display_name))}' is already enabled[/yellow]") + console.print( + f"[yellow]Extension '{_escape_markup(str(display_name))}' is already enabled[/yellow]" + ) raise typer.Exit(0) manager.registry.update(extension_id, {"enabled": True}) @@ -47,7 +50,8 @@ def extension_enable( from .. import load_init_options init_options = load_init_options(project_root) - if init_options.get("ai") == "generic": + agent = init_options.get("ai") + if agent == "generic": try: manifest = manager.get_extension(extension_id) if manifest is None: @@ -63,11 +67,17 @@ def extension_enable( manager._skill_name_for_command(command["name"]) for command in manifest.commands } - if skills else set(manager._collect_manifest_command_names(manifest)) + if skills + else set(manager._collect_manifest_command_names(manifest)) + ) + owned = set( + manager._generic_owned_names( + refreshed, + list(expected), + skills=skills, + extension_id=extension_id, + ) ) - owned = set(manager._generic_owned_names( - refreshed, list(expected), skills=skills, extension_id=extension_id, - )) missing = expected - owned if missing: manager.disable_generic_extension_artifacts(extension_id) @@ -81,6 +91,15 @@ def extension_enable( f"for '{_escape_markup(str(extension_id))}': {_escape_markup(str(exc))}" ) raise typer.Exit(1) from exc + elif agent: + # Make the enabled bit visible before refreshing extension artifacts; + # if registration fails, return to the prior disabled state. Preset + # refresh below handles selector-expanded artifacts after this succeeds. + try: + manager.register_enabled_extensions_for_agent(agent) + except Exception: + manager.registry.update(extension_id, {"enabled": False}) + raise # Enable hooks in extensions.yml config = hook_executor.get_project_config() @@ -91,11 +110,14 @@ def extension_enable( hook["enabled"] = True hook_executor.save_project_config(config) - console.print(f"[green]✓[/green] Extension '{_escape_markup(str(display_name))}' enabled") + console.print( + f"[green]✓[/green] Extension '{_escape_markup(str(display_name))}' enabled" + ) # #1: regenerate native event config so the enabled extension's events # are re-emitted in installed integrations. _commands._refresh_events_and_warn(project_root) + _commands._refresh_presets_and_warn(project_root) # Scaffold config templates on enable try: @@ -113,7 +135,9 @@ def extension_enable( for cfg in deployed: console.print(f" • {config_home}/{_escape_markup(str(cfg))}") if skipped: - console.print(f"\n[dim]Config files already exist (preserved): {_escape_markup(', '.join(skipped))}[/dim]") + console.print( + f"\n[dim]Config files already exist (preserved): {_escape_markup(', '.join(skipped))}[/dim]" + ) if failed: console.print( f"\n[yellow]Warning:[/yellow] Config templates not scaffolded: " diff --git a/src/specify_cli/extensions/command_remove.py b/src/specify_cli/extensions/command_remove.py index c210dae96b..429a5a1ad9 100644 --- a/src/specify_cli/extensions/command_remove.py +++ b/src/specify_cli/extensions/command_remove.py @@ -3,6 +3,7 @@ Registered by ``_commands.register()``; shared command infrastructure lives in ``_commands.py``. """ + from __future__ import annotations import typer @@ -15,7 +16,9 @@ @_commands.extension_app.command("remove") def extension_remove( extension: str = typer.Argument(help="Extension ID or name to remove"), - keep_config: bool = typer.Option(False, "--keep-config", help="Don't remove config files"), + keep_config: bool = typer.Option( + False, "--keep-config", help="Don't remove config files" + ), force: bool = typer.Option(False, "--force", help="Skip confirmation"), ): """Uninstall an extension.""" @@ -40,7 +43,9 @@ def extension_remove( # think in logical commands, not per-agent file counts. # Use get() without a default so we can distinguish "key missing" (fall back # to manifest) from "key present but empty dict" (zero commands registered). - registered_commands = reg_meta.get("registered_commands") if isinstance(reg_meta, dict) else None + registered_commands = ( + reg_meta.get("registered_commands") if isinstance(reg_meta, dict) else None + ) if isinstance(registered_commands, dict): cmd_count = max( (len(v) for v in registered_commands.values() if isinstance(v, list)), @@ -54,10 +59,14 @@ def extension_remove( # Confirm removal if not force: console.print("\n[yellow]⚠ This will remove:[/yellow]") - console.print(f" • {cmd_count} command{'s' if cmd_count != 1 else ''} per agent") + console.print( + f" • {cmd_count} command{'s' if cmd_count != 1 else ''} per agent" + ) if skill_count: console.print(f" • {skill_count} agent skill(s)") - console.print(f" • Extension directory: .specify/extensions/{safe_extension_id}/") + console.print( + f" • Extension directory: .specify/extensions/{safe_extension_id}/" + ) if not keep_config: console.print(" • Config files (will be backed up)") console.print() @@ -71,15 +80,22 @@ def extension_remove( success = manager.remove(extension_id, keep_config=keep_config) if success: - console.print(f"\n[green]✓[/green] Extension '{_escape_markup(str(display_name))}' removed successfully") + console.print( + f"\n[green]✓[/green] Extension '{_escape_markup(str(display_name))}' removed successfully" + ) if keep_config: - console.print(f"\nConfig files preserved in .specify/extensions/{safe_extension_id}/") + console.print( + f"\nConfig files preserved in .specify/extensions/{safe_extension_id}/" + ) else: - console.print(f"\nConfig files backed up to .specify/extensions/.backup/{safe_extension_id}/") + console.print( + f"\nConfig files backed up to .specify/extensions/.backup/{safe_extension_id}/" + ) # #1: regenerate native event config so the removed extension's events # are stripped from installed integrations. _commands._refresh_events_and_warn(project_root) + _commands._refresh_presets_and_warn(project_root) console.print(f"\nTo reinstall: specify extension add {safe_extension_id}") else: console.print("[red]Error:[/red] Failed to remove extension") diff --git a/src/specify_cli/extensions/command_set_priority.py b/src/specify_cli/extensions/command_set_priority.py index 3adbe73c9b..d6415742f9 100644 --- a/src/specify_cli/extensions/command_set_priority.py +++ b/src/specify_cli/extensions/command_set_priority.py @@ -3,6 +3,7 @@ Registered by ``_commands.register()``; shared command infrastructure lives in ``_commands.py``. """ + from __future__ import annotations import typer @@ -23,7 +24,9 @@ def extension_set_priority( project_root = _commands._require_specify_project() # Validate priority if priority < 1: - console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)") + console.print( + "[red]Error:[/red] Priority must be a positive integer (1 or higher)" + ) raise typer.Exit(1) manager = ExtensionManager(project_root) @@ -54,7 +57,9 @@ def extension_set_priority( and not isinstance(raw_priority, bool) and raw_priority == priority ): - console.print(f"[yellow]Extension '{_escape_markup(str(display_name))}' already has priority {priority}[/yellow]") + console.print( + f"[yellow]Extension '{_escape_markup(str(display_name))}' already has priority {priority}[/yellow]" + ) raise typer.Exit(0) old_priority = normalize_priority(raw_priority) @@ -62,5 +67,13 @@ def extension_set_priority( # Update priority manager.registry.update(extension_id, {"priority": priority}) - console.print(f"[green]✓[/green] Extension '{_escape_markup(str(display_name))}' priority changed: {old_priority} → {priority}") - console.print("\n[dim]Lower priority = higher precedence in template resolution[/dim]") + # Extension reordering can change the lower-layer candidates matched by + # enabled preset regex selectors. + _commands._refresh_presets_and_warn(project_root) + + console.print( + f"[green]✓[/green] Extension '{_escape_markup(str(display_name))}' priority changed: {old_priority} → {priority}" + ) + console.print( + "\n[dim]Lower priority = higher precedence in template resolution[/dim]" + ) diff --git a/src/specify_cli/integrations/_command_upgrade_layout.py b/src/specify_cli/integrations/_command_upgrade_layout.py index caf4ba52d1..fd1dd272ba 100644 --- a/src/specify_cli/integrations/_command_upgrade_layout.py +++ b/src/specify_cli/integrations/_command_upgrade_layout.py @@ -5,6 +5,7 @@ import json from pathlib import Path, PurePath + def _manifest_tracks_skill_layout(manifest) -> bool: """Return True when *manifest* tracks any skills-layout artifact. @@ -93,6 +94,7 @@ def _installed_presets_affecting_agent( agent_key: str, *, include_skills: bool = True, + include_disabled: bool = True, ) -> list[str]: """Return IDs of installed presets with artifacts registered for *agent_key*. @@ -128,9 +130,7 @@ def _installed_presets_affecting_agent( except (OSError, ValueError) as exc: raise _PresetRegistryUnreadableError(str(exc)) from exc if not isinstance(data, dict) or not isinstance(data.get("presets", {}), dict): - raise _PresetRegistryUnreadableError( - "preset registry structure is malformed" - ) + raise _PresetRegistryUnreadableError("preset registry structure is malformed") affected: list[str] = [] for preset_id, meta in data.get("presets", {}).items(): @@ -147,21 +147,23 @@ def _installed_presets_affecting_agent( raise _PresetRegistryUnreadableError( f"preset '{preset_id}' registered_commands is malformed" ) + if not meta.get("enabled", True): + # Disabled presets are only relevant to callers that explicitly + # request installed-but-disabled provenance. + if include_disabled: + affected.append(preset_id) + continue registered_skills = meta.get("registered_skills", []) if isinstance(registered_skills, dict): # Per-agent provenance ({agent: [skill names]}): only entries for # *this* agent make the preset affect it. Values must be lists — # anything else (e.g. null) leaves ownership undecidable, so fail # closed rather than read it as "no artifacts". - if not all( - isinstance(names, list) for names in registered_skills.values() - ): + if not all(isinstance(names, list) for names in registered_skills.values()): raise _PresetRegistryUnreadableError( f"preset '{preset_id}' registered_skills is malformed" ) - has_skills = include_skills and bool( - registered_skills.get(agent_key) - ) + has_skills = include_skills and bool(registered_skills.get(agent_key)) elif isinstance(registered_skills, (list, tuple)): # Legacy flat list: not agent-scoped, so any recorded skill may # belong to this agent — fail closed and count it as affecting. @@ -185,4 +187,5 @@ def _installed_command_presets_affecting_agent( project_root, agent_key, include_skills=False, + include_disabled=False, ) diff --git a/src/specify_cli/integrations/command_upgrade.py b/src/specify_cli/integrations/command_upgrade.py index b4682b6825..06a7de9054 100644 --- a/src/specify_cli/integrations/command_upgrade.py +++ b/src/specify_cli/integrations/command_upgrade.py @@ -1,4 +1,5 @@ """The ``specify integration upgrade`` command and its layout guards.""" + from __future__ import annotations import os @@ -12,7 +13,10 @@ invoke_separator_for_integration as _invoke_separator_for_integration, with_integration_setting as _with_integration_setting, ) -from ..integration_state import default_integration_key as _default_integration_key, installed_integration_keys as _installed_integration_keys +from ..integration_state import ( + default_integration_key as _default_integration_key, + installed_integration_keys as _installed_integration_keys, +) from ._command_upgrade_layout import ( _PresetRegistryUnreadableError, _installed_command_presets_affecting_agent, @@ -22,15 +26,41 @@ _manifest_tracks_skill_layout, ) from ._commands import integration_app -from ._helpers import _MANIFEST_READ_ERRORS, _SharedTemplateRefreshError, _cli_error_detail, _cli_phase_label, _get_speckit_version, _read_integration_json, _refresh_init_options_speckit_version, _register_extensions_for_agent, _register_presets_for_agent, _resolve_integration_options, _resolve_integration_script_type, _resync_manifest_after_registration, _unregister_enabled_extension_commands_for_agent, _update_init_options_for_integration, _write_integration_json +from ._helpers import ( + _MANIFEST_READ_ERRORS, + _SharedTemplateRefreshError, + _cli_error_detail, + _cli_phase_label, + _get_speckit_version, + _read_integration_json, + _refresh_init_options_speckit_version, + _register_extensions_for_agent, + _register_presets_for_agent, + _resolve_integration_options, + _resolve_integration_script_type, + _resync_manifest_after_registration, + _unregister_enabled_extension_commands_for_agent, + _update_init_options_for_integration, + _write_integration_json, +) @integration_app.command("upgrade") def integration_upgrade( - key: str | None = typer.Argument(None, help="Integration key to upgrade (default: current integration)"), - force: bool = typer.Option(False, "--force", help="Force upgrade even if files are modified"), - script: str | None = typer.Option(None, "--script", help="Script type: sh, ps, or py (default: from init-options.json or platform default)"), - integration_options: str | None = typer.Option(None, "--integration-options", help="Options for the integration"), + key: str | None = typer.Argument( + None, help="Integration key to upgrade (default: current integration)" + ), + force: bool = typer.Option( + False, "--force", help="Force upgrade even if files are modified" + ), + script: str | None = typer.Option( + None, + "--script", + help="Script type: sh, ps, or py (default: from init-options.json or platform default)", + ), + integration_options: str | None = typer.Option( + None, "--integration-options", help="Options for the integration" + ), ): """Upgrade an integration by reinstalling with diff-aware file handling. @@ -39,7 +69,11 @@ def integration_upgrade( """ from . import get_integration from .manifest import IntegrationManifest - from .. import _require_specify_project, _install_shared_infra_or_exit, _install_shared_infra + from .. import ( + _require_specify_project, + _install_shared_infra_or_exit, + _install_shared_infra, + ) project_root = _require_specify_project() current = _read_integration_json(project_root) @@ -63,26 +97,38 @@ def integration_upgrade( manifest_path = project_root / ".specify" / "integrations" / f"{key}.manifest.json" if not manifest_path.exists(): - console.print(f"[yellow]No manifest found for integration '{key}'. Nothing to upgrade.[/yellow]") - console.print(f"Run [cyan]specify integration install {key}[/cyan] to perform a fresh install.") + console.print( + f"[yellow]No manifest found for integration '{key}'. Nothing to upgrade.[/yellow]" + ) + console.print( + f"Run [cyan]specify integration install {key}[/cyan] to perform a fresh install." + ) raise typer.Exit(0) try: old_manifest = IntegrationManifest.load(key, project_root) except _MANIFEST_READ_ERRORS as exc: - console.print(f"[red]Error:[/red] Integration manifest for '{key}' is unreadable: {exc}") + console.print( + f"[red]Error:[/red] Integration manifest for '{key}' is unreadable: {exc}" + ) raise typer.Exit(1) # Detect modified files via manifest hashes modified = old_manifest.check_modified() if modified and not force: - console.print(f"[yellow]⚠[/yellow] {len(modified)} file(s) have been modified since installation:") + console.print( + f"[yellow]⚠[/yellow] {len(modified)} file(s) have been modified since installation:" + ) for rel in modified: console.print(f" {rel}") - console.print("\nUse [cyan]--force[/cyan] to overwrite modified files, or resolve manually.") + console.print( + "\nUse [cyan]--force[/cyan] to overwrite modified files, or resolve manually." + ) raise typer.Exit(1) - selected_script = _resolve_integration_script_type(project_root, current, key, script) + selected_script = _resolve_integration_script_type( + project_root, current, key, script + ) # Build parsed options from --integration-options so the integration # can determine its effective invoke separator before shared infra @@ -154,7 +200,9 @@ def integration_upgrade( parsed_options, project_root ): try: - affected_presets = _installed_presets_affecting_agent(project_root, key) + affected_presets = _installed_presets_affecting_agent( + project_root, key, include_disabled=True + ) except _PresetRegistryUnreadableError as exc: console.print( f"[red]Error:[/red] Cannot change '{key}' command layout: the " @@ -183,7 +231,7 @@ def integration_upgrade( "Remove the preset(s), run the upgrade, then reinstall them:\n" f" [cyan]specify preset remove [/cyan]\n" f" [cyan]specify integration upgrade {key} " - f"--integration-options \"...\"[/cyan]\n" + f'--integration-options "..."[/cyan]\n' f" [cyan]specify preset add [/cyan]" ) raise typer.Exit(1) @@ -205,7 +253,10 @@ def integration_upgrade( selected_script, force=force, invoke_separator=_invoke_separator_for_integration( - infra_integration, current, infra_key, infra_parsed, + infra_integration, + current, + infra_key, + infra_parsed, project_root=project_root, ), invoke_prefix=_invoke_prefix_for_integration( @@ -214,13 +265,17 @@ def integration_upgrade( ) if os.name != "nt": from .. import ensure_executable_scripts + ensure_executable_scripts(project_root) # Phase 1: Install new files (overwrites existing; old-only files remain) console.print(f"Upgrading integration: [cyan]{key}[/cyan]") - new_manifest = IntegrationManifest(key, project_root, version=_get_speckit_version()) + new_manifest = IntegrationManifest( + key, project_root, version=_get_speckit_version() + ) from ..events import resolve_events + events_map = resolve_events( key, integration.config, @@ -251,7 +306,10 @@ def integration_upgrade( project_root, selected_script, invoke_separator=_invoke_separator_for_integration( - integration, {"integration_settings": settings}, key, parsed_options, + integration, + {"integration_settings": settings}, + key, + parsed_options, project_root=project_root, ), invoke_prefix=_invoke_prefix_for_integration( @@ -266,6 +324,7 @@ def integration_upgrade( ) from exc if os.name != "nt": from .. import ensure_executable_scripts + ensure_executable_scripts(project_root) new_manifest.save() _write_integration_json(project_root, installed_key, installed_keys, settings) @@ -281,9 +340,13 @@ def integration_upgrade( except Exception as exc: # Don't teardown — setup overwrites in-place, so teardown would # delete files that were working before the upgrade. Just report. - console.print(f"[red]Error:[/red] Failed to {_cli_phase_label('upgrade', 'integration', key)}.") + console.print( + f"[red]Error:[/red] Failed to {_cli_phase_label('upgrade', 'integration', key)}." + ) console.print(f"[dim]Details:[/dim] {_cli_error_detail(exc)}") - console.print("[yellow]The previous integration files may still be in place.[/yellow]") + console.print( + "[yellow]The previous integration files may still be in place.[/yellow]" + ) raise typer.Exit(1) # Phase 2: Remove stale files from old manifest that are not in the new one @@ -294,7 +357,9 @@ def integration_upgrade( # as "stale" while still being actively managed. Manifest keys are stored # in POSIX form, so normalize the exclusions the same way before subtracting # (an integration may build paths with os.path.join / backslashes). - exclusions = {PurePath(p).as_posix() for p in integration.stale_cleanup_exclusions()} + exclusions = { + PurePath(p).as_posix() for p in integration.stale_cleanup_exclusions() + } stale_keys = (set(old_files) - set(new_files)) - exclusions if stale_keys: stale_manifest = IntegrationManifest(key, project_root, version="stale-cleanup") @@ -309,7 +374,9 @@ def integration_upgrade( project_root, force=True, remove_manifest=False ) if stale_removed: - console.print(f" Removed {len(stale_removed)} stale file(s) from previous install") + console.print( + f" Removed {len(stale_removed)} stale file(s) from previous install" + ) legacy_command_root_changed = _legacy_command_root_changed( integration, diff --git a/src/specify_cli/presets/_manager.py b/src/specify_cli/presets/_manager.py index 33274dfdfc..354415f0b8 100644 --- a/src/specify_cli/presets/_manager.py +++ b/src/specify_cli/presets/_manager.py @@ -2,8 +2,11 @@ import hashlib import json +import os import shutil +import copy import tempfile +import uuid from pathlib import Path from typing import Any, Dict, List, Optional, Set @@ -30,11 +33,31 @@ ) from ._registry import PresetRegistry from ._resolver import PresetResolver +from ._selectors import is_regex_selector, selector_matches _CONSTITUTION_PROVENANCE_FILE = ".constitution-template.json" _CONSTITUTION_SYNC_PRESET_ID = "constitution-sync" +class _ExpandedCommandManifest: + """Manifest view exposing concrete command selector expansions.""" + + def __init__(self, manifest: PresetManifest, commands: List[Dict[str, Any]]): + self._manifest = manifest + self._commands = commands + + def __getattr__(self, name: str): + return getattr(self._manifest, name) + + @property + def templates(self) -> List[Dict[str, Any]]: + return [ + template + for template in self._manifest.templates + if template.get("type") != "command" + ] + self._commands + + def _content_sha256(content: bytes) -> str: return hashlib.sha256(content).hexdigest() @@ -69,16 +92,13 @@ def _constitution_is_generated( metadata = json.loads(provenance.read_text(encoding="utf-8")) except (json.JSONDecodeError, UnicodeDecodeError): return False - return ( - isinstance(metadata, dict) - and metadata.get("sha256") == _content_sha256(content) + return isinstance(metadata, dict) and metadata.get("sha256") == _content_sha256( + content ) # Older projects have no provenance sidecar. Only the immutable bundled or # source-checkout core template is safe to treat as generated. - core = resolver._find_bundled_core( - "constitution-template", "template", ".md" - ) + core = resolver._find_bundled_core("constitution-template", "template", ".md") return core is not None and core.read_bytes() == content @@ -164,9 +184,7 @@ def __init__(self, project_root: Path): self.registry = PresetRegistry(self.presets_dir) def check_compatibility( - self, - manifest: PresetManifest, - speckit_version: str + self, manifest: PresetManifest, speckit_version: str ) -> bool: """Check if preset is compatible with current spec-kit version. @@ -208,8 +226,7 @@ def check_compatibility( return True def find_unmet_extension_dependencies( - self, - manifest: PresetManifest + self, manifest: PresetManifest ) -> List[Dict[str, Any]]: """Find declared extension dependencies that are not satisfied. @@ -321,9 +338,7 @@ def find_unmet_extension_dependencies( # the preset is as inert as if it were never installed -- but the # surviving entry would otherwise read as satisfied. if not (extensions_dir / dep["id"]).is_dir(): - unmet.append( - {**dep, "installed": installed_version, "reason": "stale"} - ) + unmet.append({**dep, "installed": installed_version, "reason": "stale"}) continue # A disabled extension is registered but contributes nothing: @@ -347,7 +362,9 @@ def find_unmet_extension_dependencies( # apart -- it catches InvalidVersion and returns False, which would # report a mismatch against a version nobody can evaluate. Check # parseability up front so only real comparisons reach the warning. - if installed_version is None or not _is_comparable_version(installed_version): + if installed_version is None or not _is_comparable_version( + installed_version + ): continue if not version_satisfies(installed_version, constraint): unmet.append( @@ -382,26 +399,57 @@ def install_from_directory( """ # Validate priority if priority < 1: - raise PresetValidationError("Priority must be a positive integer (1 or higher)") + raise PresetValidationError( + "Priority must be a positive integer (1 or higher)" + ) manifest_path = source_dir / "preset.yml" manifest = PresetManifest(manifest_path) self.check_compatibility(manifest, speckit_version) - if self.registry.is_installed(manifest.id): + was_installed = self.registry.is_installed(manifest.id) + if was_installed: if not force: raise PresetError( f"Preset '{manifest.id}' is already installed. " f"Use 'specify preset remove {manifest.id}' first." ) - self.remove(manifest.id) dest_dir = self.presets_dir / manifest.id - if dest_dir.exists(): - shutil.rmtree(dest_dir) + self.presets_dir.mkdir(parents=True, exist_ok=True) + stage_dir = self.presets_dir / f".{manifest.id}.stage-{uuid.uuid4().hex}" + try: + # Copy before touching the current install. A failed copy therefore + # cannot destroy either an existing preset or leave a partial one. + shutil.copytree(source_dir, stage_dir) + except Exception: + shutil.rmtree(stage_dir, ignore_errors=True) + raise - shutil.copytree(source_dir, dest_dir) + previous_metadata = self.registry.get(manifest.id) if was_installed else None + previous_registry_data = copy.deepcopy(self.registry.data) + backup_dir = self.presets_dir / f".{manifest.id}.backup-{uuid.uuid4().hex}" + moved_old_dir = False + try: + if was_installed and dest_dir.exists(): + # Keep an independent rollback image while remove() performs + # its established artifact cleanup and registry transition. + shutil.copytree(dest_dir, backup_dir) + self.remove(manifest.id) + moved_old_dir = True + if dest_dir.exists(): + os.replace(dest_dir, backup_dir) + moved_old_dir = True + os.replace(stage_dir, dest_dir) + except Exception: + shutil.rmtree(stage_dir, ignore_errors=True) + if moved_old_dir and backup_dir.exists(): + os.replace(backup_dir, dest_dir) + if previous_metadata is not None: + self.registry.data = previous_registry_data + self.registry._save() + raise # Pre-register the preset so that composition resolution can see it # in the priority stack when resolving composed command content. @@ -413,37 +461,82 @@ def install_from_directory( if normalized_catalog_name else "local" ) - self.registry.add(manifest.id, { - "version": manifest.version, - "source": source, - "manifest_hash": manifest.get_hash(), - "enabled": True, - "priority": priority, - "registered_commands": {}, - "registered_skills": {}, - }) + try: + self.registry.add( + manifest.id, + { + "version": manifest.version, + "source": source, + "manifest_hash": manifest.get_hash(), + "enabled": True, + "priority": priority, + "registered_commands": {}, + "registered_skills": {}, + }, + ) + except Exception: + if dest_dir.exists(): + shutil.rmtree(dest_dir, ignore_errors=True) + if moved_old_dir and backup_dir.exists(): + os.replace(backup_dir, dest_dir) + self.registry.data = previous_registry_data + self.registry._save() + # Restore materialized artifacts from the restored preset stack. + if previous_metadata is not None: + try: + old_manifest = PresetManifest(dest_dir / "preset.yml") + old_commands = sorted( + item["name"] + for item in old_manifest.templates + if item.get("type") == "command" + and isinstance(item.get("name"), str) + and not is_regex_selector(item["name"]) + ) + if old_commands: + self._reconcile_composed_commands(old_commands) + self._reconcile_skills(old_commands) + except Exception: + pass + raise + raw_command_templates = [ + template + for template in manifest.templates + if template.get("type") == "command" + ] registered_commands: Dict[str, List[str]] = {} registered_skills: Dict[str, List[str]] = {} + command_templates: List[Dict[str, Any]] = [] try: - # Register command overrides with AI agents and persist the result - # immediately so cleanup can recover even if installation stops - # before later phases complete. - registered_commands = self._register_commands(manifest, dest_dir) - self.registry.update(manifest.id, { - "registered_commands": registered_commands, - }) + self._warn_unmatched_resource_selectors(manifest, dest_dir) + command_templates = self._expand_command_selectors( + PresetResolver(self.project_root), dest_dir, raw_command_templates + ) + # Register both command artifacts and skills from the same expanded + # concrete command declaration set. + registered_commands = self._register_commands( + manifest, dest_dir, command_templates=command_templates + ) + self.registry.update( + manifest.id, + {"registered_commands": registered_commands}, + ) # Update corresponding skills when skills mode was previously used # and persist that result as well. - registered_skills = self._register_skills(manifest, dest_dir) - self.registry.update(manifest.id, { - "registered_skills": registered_skills, - }) + registered_skills = self._register_skills( + manifest, dest_dir, command_templates=command_templates + ) + self.registry.update( + manifest.id, + { + "registered_skills": registered_skills, + }, + ) except Exception: - # Roll back all side effects. _register_skills persists each - # successful write immediately, so reload that partial map when - # a later template fails before the call can return. + # Roll back artifacts produced by the failed install. Registration + # helpers may persist partial metadata, so restore the full snapshot + # rather than removing only the new registry entry. if registered_commands: self._unregister_commands(registered_commands) persisted_metadata = self.registry.get(manifest.id) or {} @@ -454,27 +547,61 @@ def install_from_directory( self._unregister_skills( persisted_skills, dest_dir, restore_from_bundled_core=True ) - try: - if dest_dir.exists(): - shutil.rmtree(dest_dir) - except OSError: - pass # best-effort cleanup; don't mask the original error - self.registry.remove(manifest.id) + if dest_dir.exists(): + shutil.rmtree(dest_dir, ignore_errors=True) + if moved_old_dir and backup_dir.exists(): + os.replace(backup_dir, dest_dir) + self.registry.data = previous_registry_data + self.registry._save() + + # Re-materialize artifacts using the restored preset stack. Keep + # the backup until this succeeds so rollback has the old sources. + if previous_metadata is not None: + try: + old_manifest = PresetManifest(dest_dir / "preset.yml") + old_templates = [ + item for item in old_manifest.templates + if item.get("type") == "command" + ] + old_commands = self._expand_command_selectors( + PresetResolver(self.project_root), dest_dir, old_templates + ) + old_names = sorted({ + item["name"] for item in old_commands + if isinstance(item.get("name"), str) + and not is_regex_selector(item["name"]) + }) + if old_names: + self._reconcile_composed_commands(old_names) + self._reconcile_skills(old_names) + except Exception: + pass raise - # Reconcile all affected commands from the full priority stack so that - # install order doesn't determine the winning command file. - cmd_names = [ - t["name"] - for t in manifest.templates - if t.get("type") == "command" + command_templates = [ + template + for template in manifest.templates + if template.get("type") == "command" ] + expanded_templates = self._expand_command_selectors( + PresetResolver(self.project_root), dest_dir, command_templates + ) + cmd_names = sorted( + { + item["name"] + for item in expanded_templates + if isinstance(item.get("name"), str) + and not is_regex_selector(item["name"]) + } + ) + if cmd_names: try: self._reconcile_composed_commands(cmd_names) self._reconcile_skills(cmd_names) except Exception as exc: import warnings + warnings.warn( f"Post-install reconciliation failed for {manifest.id}: {exc}. " f"Agent command files may not reflect the current priority stack.", @@ -490,8 +617,81 @@ def install_from_directory( # previous install-time behavior for teams that want reviewed snapshots. self._seed_constitution_from_preset(manifest, dest_dir) + if moved_old_dir: + shutil.rmtree(backup_dir) + return manifest + def _warn_unmatched_resource_selectors( + self, manifest: PresetManifest, preset_dir: Path + ) -> None: + """Warn, without persisting state, for selectors with no lower-layer match.""" + import warnings + + from ._selectors import is_regex_selector, selector_matches + + resolver = PresetResolver(self.project_root) + for declaration in manifest.templates: + selector = declaration.get("name") + resource_type = declaration.get("type") + if not isinstance(selector, str) or not is_regex_selector(selector): + continue + if resource_type == "command": + matches = self._expand_command_selectors( + resolver, preset_dir, [declaration] + ) + elif resource_type in {"template", "script"}: + from ..artifacts.catalog import ArtifactCatalog + + inventory = ArtifactCatalog(self.project_root).list_artifacts() + candidates = { + artifact.name + for artifact in inventory + if artifact.kind == resource_type + and isinstance(artifact.name, str) + and selector_matches(selector, artifact.name) + and self._has_lower_resource( + preset_dir, artifact.name, resource_type + ) + } + matches = [{"name": name} for name in sorted(candidates)] + else: + continue + if not matches: + warnings.warn( + f"Preset '{manifest.id}' selector '{selector}' currently matches no {resource_type} resources. " + "Regex selectors only match concrete resources from lower layers.", + stacklevel=2, + ) + + def _has_lower_resource( + self, preset_dir: Path, name: str, resource_type: str + ) -> bool: + try: + resolver = PresetResolver(self.project_root) + except Exception: + return False + stack = self.registry.list_by_priority() + try: + index = next( + i for i, (pid, _meta) in enumerate(stack) if pid == preset_dir.name + ) + except StopIteration: + return False + if any( + resolver._has_concrete_resource(self.presets_dir / pid, name, resource_type) + for pid, _meta in stack[index + 1 :] + ): + return True + if any( + resolver._extension_has_concrete_resource( + resolver.extensions_dir / ext_id, name, resource_type + ) + for _priority, ext_id, _meta in resolver._get_all_extensions_by_priority() + ): + return True + return resolver._core_has_concrete_resource(name, resource_type) + def _seed_constitution_from_preset( self, manifest: PresetManifest, preset_dir: Path ) -> None: @@ -501,14 +701,30 @@ def _seed_constitution_from_preset( stack. Later preset installs only reconcile when they provide a ``constitution-template``. Authored constitutions are never overwritten. """ - provides_constitution = manifest.id == _CONSTITUTION_SYNC_PRESET_ID or any( - t.get("type") == "template" and t.get("name") == "constitution-template" - for t in manifest.templates - ) or any( - (preset_dir / relative_path).is_file() - for relative_path in ( - "templates/constitution-template.md", - "constitution-template.md", + provides_constitution = ( + manifest.id == _CONSTITUTION_SYNC_PRESET_ID + or any( + declaration.get("type") == "template" + and isinstance(declaration.get("name"), str) + and declaration.get("name") == "constitution-template" + for declaration in manifest.templates + ) + or any( + declaration.get("type") == "template" + and isinstance(declaration.get("name"), str) + and is_regex_selector(declaration["name"]) + and selector_matches(declaration["name"], "constitution-template") + and self._has_lower_resource( + preset_dir, "constitution-template", "template" + ) + for declaration in manifest.templates + ) + or any( + (preset_dir / relative_path).is_file() + for relative_path in ( + "templates/constitution-template.md", + "constitution-template.md", + ) ) ) if not provides_constitution: @@ -577,7 +793,9 @@ def install_from_archive( """ # Validate priority early if priority < 1: - raise PresetValidationError("Priority must be a positive integer (1 or higher)") + raise PresetValidationError( + "Priority must be a positive integer (1 or higher)" + ) with tempfile.TemporaryDirectory() as tmpdir: temp_path = Path(tmpdir) @@ -598,9 +816,7 @@ def install_from_archive( manifest_path = pack_dir / "preset.yml" if not manifest_path.exists(): - raise PresetValidationError( - "No preset.yml found in archive" - ) + raise PresetValidationError("No preset.yml found in archive") return self.install_from_directory( pack_dir, @@ -659,7 +875,9 @@ def remove(self, pack_id: str) -> bool: from .. import load_init_options init_opts = load_init_options(self.project_root) - fallback_agent = init_opts.get("ai") if isinstance(init_opts, dict) else None + fallback_agent = ( + init_opts.get("ai") if isinstance(init_opts, dict) else None + ) if not isinstance(fallback_agent, str): fallback_agent = "" registered_skills = self._infer_legacy_skill_provenance( @@ -667,7 +885,9 @@ def remove(self, pack_id: str) -> bool: pack_id, fallback_agent=fallback_agent, ) - registered_commands = metadata.get("registered_commands", {}) if metadata else {} + registered_commands = ( + metadata.get("registered_commands", {}) if metadata else {} + ) pack_dir = self.presets_dir / pack_id # Record which historical agents this preset's registered_commands @@ -688,19 +908,13 @@ def remove(self, pack_id: str) -> bool: agent_name for agent_name in registered_commands if _CommandRegistrarForScope is None - or _CommandRegistrarForScope.AGENT_CONFIGS.get(agent_name, {}).get("extension") != "/SKILL.md" + or _CommandRegistrarForScope.AGENT_CONFIGS.get(agent_name, {}).get( + "extension" + ) + != "/SKILL.md" } - # Collect ALL command names before filtering for reconciliation, - # so commands registered only for skill-based agents are also - # reconciled. Every command-type template's primary name is added - # unconditionally (not just aliases) since ai_skills-mode presets - # never populate registered_commands for command-backed - # integrations (see _register_commands's ai_skills guard) — without - # this, removing a skills-mode preset that overrides a command no - # other preset registered "the normal way" would skip reconciliation - # entirely and _unregister_skills would restore core/extension - # content instead of a surviving lower-priority preset's override. + # Collect ALL command names before filtering for reconciliation. removed_cmd_names = set() removed_constitution = any( path.exists() @@ -721,33 +935,46 @@ def remove(self, pack_id: str) -> bool: metadata["version"], ) ) - for cmd_names in registered_commands.values(): - removed_cmd_names.update(cmd_names) manifest_path = pack_dir / "preset.yml" if manifest_path.exists(): try: manifest = PresetManifest(manifest_path) + from ._manager_commands import _PresetCommandMethods + + resolver = PresetResolver(self.project_root) for tmpl in manifest.templates: + if tmpl.get("type") == "command": + name = tmpl.get("name") + if isinstance(name, str): + expanded = ( + _PresetCommandMethods._expand_command_selectors( + self, resolver, pack_dir, [tmpl] + ) + if is_regex_selector(name) + else [tmpl] + ) + removed_cmd_names.update( + item["name"] + for item in expanded + if isinstance(item.get("name"), str) + and not is_regex_selector(item["name"]) + ) + removed_cmd_names.update( + alias + for alias in tmpl.get("aliases", []) + if isinstance(alias, str) + ) if ( tmpl.get("type") == "template" and tmpl.get("name") == "constitution-template" ): removed_constitution = True - if tmpl.get("type") == "command": - name = tmpl.get("name") - if isinstance(name, str): - removed_cmd_names.add(name) - for alias in tmpl.get("aliases", []): - if isinstance(alias, str): - removed_cmd_names.add(alias) except PresetValidationError: - # Invalid manifest — skip alias extraction; primary command - # names from registered_commands are still unregistered. pass + for cmd_names in registered_commands.values(): + removed_cmd_names.update(cmd_names) - affected_skill_dirs: Dict[ - Path, tuple[Optional[str], List[str]] - ] = {} + affected_skill_dirs: Dict[Path, tuple[Optional[str], List[str]]] = {} if registered_skills: restorable_skills = registered_skills # A skill tracked for a command-backed agent whose ai_skills is @@ -764,9 +991,7 @@ def remove(self, pack_id: str) -> bool: # shadowing the module-level name for this whole function. from .._init_options import load_init_options as _load_init_options - resolved_active = resolve_active_agent_for_registration( - self.project_root - ) + resolved_active = resolve_active_agent_for_registration(self.project_root) if ( isinstance(registered_skills, dict) and isinstance(resolved_active, str) @@ -774,17 +999,14 @@ def remove(self, pack_id: str) -> bool: and _CommandRegistrarForScope is not None and _CommandRegistrarForScope.AGENT_CONFIGS.get( resolved_active, {} - ).get("extension") != "/SKILL.md" - and not is_ai_skills_enabled( - _load_init_options(self.project_root) - ) + ).get("extension") + != "/SKILL.md" + and not is_ai_skills_enabled(_load_init_options(self.project_root)) ): raw_names = registered_skills.get(resolved_active) stale_names = [ name - for name in ( - raw_names if isinstance(raw_names, list) else [] - ) + for name in (raw_names if isinstance(raw_names, list) else []) if isinstance(name, str) ] restorable_skills = { @@ -813,16 +1035,14 @@ def remove(self, pack_id: str) -> bool: CommandRegistrar = None if CommandRegistrar is not None: skill_coverage = ( - registered_skills - if isinstance(registered_skills, dict) - else {} + registered_skills if isinstance(registered_skills, dict) else {} ) commands_to_unregister: Dict[str, List[str]] = {} for agent_name, cmd_names in registered_commands.items(): is_native_skill_agent = ( - CommandRegistrar.AGENT_CONFIGS.get( - agent_name, {} - ).get("extension") + CommandRegistrar.AGENT_CONFIGS.get(agent_name, {}).get( + "extension" + ) == "/SKILL.md" ) if not is_native_skill_agent: @@ -833,9 +1053,7 @@ def remove(self, pack_id: str) -> bool: covered_skill_names = { name for name in ( - raw_skill_names - if isinstance(raw_skill_names, list) - else [] + raw_skill_names if isinstance(raw_skill_names, list) else [] ) if isinstance(name, str) } @@ -848,9 +1066,7 @@ def remove(self, pack_id: str) -> bool: ) ] if uncovered_commands: - commands_to_unregister[agent_name] = ( - uncovered_commands - ) + commands_to_unregister[agent_name] = uncovered_commands registered_commands = commands_to_unregister # Unregister non-skill command files from AI agents. @@ -874,6 +1090,7 @@ def remove(self, pack_id: str) -> bool: ) except Exception as exc: import warnings + warnings.warn( f"Post-removal reconciliation failed for {pack_id}: {exc}. " f"Agent command files may be stale; reinstall affected presets " @@ -884,7 +1101,12 @@ def remove(self, pack_id: str) -> bool: if removed_constitution: try: self._reconcile_constitution() - except (OSError, UnicodeDecodeError, PresetValidationError, ValueError) as exc: + except ( + OSError, + UnicodeDecodeError, + PresetValidationError, + ValueError, + ) as exc: import warnings warnings.warn( @@ -912,39 +1134,55 @@ def list_installed(self) -> List[Dict[str, Any]]: try: manifest = PresetManifest(manifest_path) - provided_counts = {"commands": 0, "templates": 0, "scripts": 0, "hooks": 0} + provided_counts = { + "commands": 0, + "templates": 0, + "scripts": 0, + "hooks": 0, + } for template in manifest.templates: provided_counts[f"{template['type']}s"] += 1 author = manifest.author - result.append({ - "id": pack_id, - "name": manifest.name, - "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": normalize_priority(metadata.get("priority")), - "_json_author": author if isinstance(author, str) and author else None, - "_json_source": metadata.get("source"), - "_json_provides": provided_counts, - }) + result.append( + { + "id": pack_id, + "name": manifest.name, + "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": normalize_priority(metadata.get("priority")), + "_json_author": author + if isinstance(author, str) and author + else None, + "_json_source": metadata.get("source"), + "_json_provides": provided_counts, + } + ) except PresetValidationError: - result.append({ - "id": pack_id, - "name": pack_id, - "version": metadata.get("version", "unknown"), - "description": "⚠️ Corrupted preset", - "enabled": False, - "installed_at": metadata.get("installed_at"), - "template_count": 0, - "tags": [], - "priority": normalize_priority(metadata.get("priority")), - "_json_author": None, - "_json_source": metadata.get("source"), - "_json_provides": {"commands": 0, "templates": 0, "scripts": 0, "hooks": 0}, - }) + result.append( + { + "id": pack_id, + "name": pack_id, + "version": metadata.get("version", "unknown"), + "description": "⚠️ Corrupted preset", + "enabled": False, + "installed_at": metadata.get("installed_at"), + "template_count": 0, + "tags": [], + "priority": normalize_priority(metadata.get("priority")), + "_json_author": None, + "_json_source": metadata.get("source"), + "_json_provides": { + "commands": 0, + "templates": 0, + "scripts": 0, + "hooks": 0, + }, + } + ) return result diff --git a/src/specify_cli/presets/_manager_commands.py b/src/specify_cli/presets/_manager_commands.py index 4d665b45dc..90aba7e914 100644 --- a/src/specify_cli/presets/_manager_commands.py +++ b/src/specify_cli/presets/_manager_commands.py @@ -15,8 +15,9 @@ resolve_active_agent_for_registration, ) from ..extensions import ExtensionRegistry -from ._manifest import PresetManifest +from ._manifest import PresetManifest, PresetValidationError from ._resolver import PresetResolver +from ._selectors import is_regex_selector, selector_matches def _substitute_core_template( @@ -46,7 +47,7 @@ def _substitute_core_template( # Derive the short name (strip "speckit." prefix) used by core command templates. short_name = cmd_name if short_name.startswith("speckit."): - short_name = short_name[len("speckit."):] + short_name = short_name[len("speckit.") :] resolver = PresetResolver(project_root) # Resolution order for the core template: @@ -91,10 +92,141 @@ def _substitute_core_template( class _PresetCommandMethods: """Command artifact methods shared through PresetManager's lifecycle state.""" + def _collect_selector_command_names(self, resolver: PresetResolver) -> set[str]: + """Collect concrete command names referenced by installed preset declarations.""" + names: set[str] = set() + for preset_id, _metadata in self.registry.list_by_priority( + include_disabled=True + ): + preset_dir = self.presets_dir / preset_id + manifest = resolver._get_manifest(preset_dir) + if manifest is None: + continue + declarations = [ + item for item in manifest.templates if item.get("type") == "command" + ] + expanded = _PresetCommandMethods._expand_command_selectors( + self, resolver, preset_dir, declarations + ) + names.update( + item["name"] + for item in expanded + if isinstance(item.get("name"), str) + and not is_regex_selector(item["name"]) + ) + metadata = self.registry.get(preset_id) or {} + for per_agent in (metadata.get("registered_commands", {}) or {}).values(): + if isinstance(per_agent, list): + names.update(name for name in per_agent if isinstance(name, str)) + return names + + def _expand_command_selectors( + self, + resolver: PresetResolver, + preset_dir: Path, + command_templates: list[dict[str, Any]], + ) -> list[dict[str, Any]]: + """Expand command selectors to lower-layer concrete entries. + + A regex declaration is eligible only if its selector matches at least + one command contributed below the current preset; it is then replaced + by exact-name copies so existing registration/tracking sees only real + command names. + """ + presets = self.registry.list_by_priority() + current_id = preset_dir.name + current_index = next( + ( + i + for i, (preset_id, _meta) in enumerate(presets) + if preset_id == current_id + ), + None, + ) + if current_index is None: + return [ + declaration + for declaration in command_templates + if not is_regex_selector(str(declaration.get("name", ""))) + ] + lower_preset_ids = { + preset_id for preset_id, _meta in presets[current_index + 1 :] + } + lower_extension_ids = { + ext_id + for _priority, ext_id, _meta in resolver._get_all_extensions_by_priority() + } + try: + from ..artifacts.catalog import ArtifactCatalog + from ..extensions import CORE_COMMAND_NAMES + + candidates = ArtifactCatalog(self.project_root).list_artifacts() + except Exception as exc: + raise PresetValidationError( + f"Could not enumerate concrete commands for preset selector: {exc}" + ) from exc + concrete_candidates = { + artifact.name + for artifact in candidates + if artifact.kind == "command" + and isinstance(artifact.name, str) + and not is_regex_selector(artifact.name) + } + concrete_candidates.update(f"speckit.{name}" for name in CORE_COMMAND_NAMES) + concrete_names = sorted( + { + name + for name in concrete_candidates + if self._command_name_has_lower_layer( + resolver, name, lower_preset_ids, lower_extension_ids + ) + } + ) + + expanded: list[dict[str, Any]] = [] + for declaration in command_templates: + name = declaration.get("name") + if not isinstance(name, str) or not is_regex_selector(name): + expanded.append(declaration) + continue + expanded.extend( + {**declaration, "name": concrete_name} + for concrete_name in concrete_names + if selector_matches(name, concrete_name) + ) + return expanded + + @staticmethod + def _command_name_has_lower_layer( + resolver: PresetResolver, + name: str, + lower_preset_ids: set[str], + lower_extension_ids: set[str], + ) -> bool: + """Use the resolver's concrete stack, excluding project override/current preset.""" + for layer in resolver.collect_all_layers(name, "command"): + source = layer["source"] + if source == "project override": + continue + if any( + source.startswith(f"{preset_id} v") for preset_id in lower_preset_ids + ): + return True + if any( + source.startswith(f"extension:{ext_id}") + for ext_id in lower_extension_ids + ): + return True + if source in {"core", "core (bundled)"}: + return True + return False + def _register_commands( self, manifest: PresetManifest, - preset_dir: Path + preset_dir: Path, + *, + command_templates: Optional[List[Dict[str, Any]]] = None, ) -> Dict[str, List[str]]: """Register preset command overrides with all detected AI agents. @@ -113,9 +245,17 @@ def _register_commands( Returns: Dictionary mapping agent names to lists of registered command names """ - command_templates = [ + raw_command_templates = [ t for t in manifest.templates if t.get("type") == "command" ] + resolver = PresetResolver(self.project_root) + command_templates = ( + command_templates + if command_templates is not None + else self._expand_command_selectors( + resolver, preset_dir, raw_command_templates + ) + ) if not command_templates: return {} @@ -139,8 +279,8 @@ def _register_commands( # If a higher-priority replace already wins, skip composition # here — reconciliation will write the correct content. layers = resolver.collect_all_layers(cmd["name"], "command") - top_layer_is_ours = ( - layers and layers[0]["path"].is_relative_to(preset_dir) + top_layer_is_ours = layers and layers[0]["path"].is_relative_to( + preset_dir ) if top_layer_is_ours: composed = resolver.resolve_content(cmd["name"], "command") @@ -150,10 +290,12 @@ def _register_commands( composed_dir.mkdir(parents=True, exist_ok=True) composed_file = composed_dir / f"{cmd['name']}.md" composed_file.write_text(composed, encoding="utf-8") - commands_to_register.append({ - **cmd, - "file": f".composed/{cmd['name']}.md", - }) + commands_to_register.append( + { + **cmd, + "file": f".composed/{cmd['name']}.md", + } + ) else: # No base layer to compose onto (e.g. the command it # would wrap comes from an extension that isn't @@ -163,6 +305,7 @@ def _register_commands( # _reconcile_composed_commands so command-mode and # reconciliation behave identically. import warnings + warnings.warn( f"Command '{cmd['name']}' uses '{strategy}' " f"strategy but no base command layer exists to " @@ -268,14 +411,20 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: agent_config = CommandRegistrar().AGENT_CONFIGS.get(agent_name) except ImportError: agent_config = None - is_command_backed = bool(agent_config) and agent_config.get("extension") != "/SKILL.md" + is_command_backed = ( + bool(agent_config) and agent_config.get("extension") != "/SKILL.md" + ) ai_skills_now = is_command_backed and is_ai_skills_enabled( load_init_options(self.project_root) ) + from ._resolver import PresetResolver + from ._selectors import is_regex_selector + resolver = PresetResolver(self.project_root) - affected_cmd_names: set = set() + affected_cmd_names: set[str] = set() presets_by_priority = list(self.registry.list_by_priority()) + expanded_declarations_by_pack: Dict[str, List[Dict[str, Any]]] = {} winning_pack_by_command: Dict[str, str] = {} winning_source_by_command: Dict[str, Path] = {} project_override_commands: set[str] = set() @@ -285,27 +434,28 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: ) if candidate_manifest is None: continue - for template in candidate_manifest.templates: + command_templates = [ + template + for template in candidate_manifest.templates + if template.get("type") == "command" + ] + expanded_for_reconcile = self._expand_command_selectors( + resolver, self.presets_dir / candidate_pack_id, command_templates + ) + expanded_declarations_by_pack[candidate_pack_id] = expanded_for_reconcile + for template in expanded_for_reconcile: command_name = template.get("name") - if ( - template.get("type") == "command" - and isinstance(command_name, str) - ): - if ( - resolver.overrides_dir / f"{command_name}.md" - ).is_file(): - project_override_commands.add(command_name) - winning_pack_by_command.setdefault( - command_name, candidate_pack_id + if not isinstance(command_name, str) or is_regex_selector(command_name): + continue + if (resolver.overrides_dir / f"{command_name}.md").is_file(): + project_override_commands.add(command_name) + winning_pack_by_command.setdefault(command_name, candidate_pack_id) + source_file = template.get("file") + if isinstance(source_file, str): + winning_source_by_command.setdefault( + command_name, + self.presets_dir / candidate_pack_id / source_file, ) - source_file = template.get("file") - if isinstance(source_file, str): - winning_source_by_command.setdefault( - command_name, - self.presets_dir - / candidate_pack_id - / source_file, - ) pending_command_cleanups: List[ tuple[ @@ -332,32 +482,28 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: if manifest is None: continue - # Registration can write one command and then fail on a later - # template. Record names first so final reconciliation can repair - # any partial writes even when _register_commands never returns. - for tmpl in manifest.templates: + concrete_declarations = expanded_declarations_by_pack.get(pack_id, []) + for tmpl in concrete_declarations: name = tmpl.get("name") - if tmpl.get("type") == "command" and isinstance(name, str): + if isinstance(name, str) and not is_regex_selector(name): affected_cmd_names.add(name) # Isolate per-preset failures: one preset that fails to register # must not abort registration of the remaining enabled presets. try: - registered_commands = self._register_commands(manifest, pack_dir) + registered_commands = self._register_commands( + manifest, pack_dir, command_templates=concrete_declarations + ) registered_command_names = set( registered_commands.get(agent_name) or [] ) - for tmpl in manifest.templates: - if tmpl.get("type") != "command": - continue + for tmpl in concrete_declarations: primary_name = tmpl.get("name") if ( isinstance(primary_name, str) and primary_name in registered_command_names ): - successful_command_replacements.add( - (pack_id, primary_name) - ) + successful_command_replacements.add((pack_id, primary_name)) existing_commands = metadata.get("registered_commands", {}) if not isinstance(existing_commands, dict): existing_commands = {} @@ -391,13 +537,17 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: # the commands phase already wrote to disk must still be # tracked so preset removal can clean them up (#2948). if merged_commands != existing_commands: - self.registry.update(pack_id, {"registered_commands": merged_commands}) + self.registry.update( + pack_id, {"registered_commands": merged_commands} + ) - registered_skills = self._register_skills(manifest, pack_dir) + registered_skills = self._register_skills( + manifest, + pack_dir, + command_templates=concrete_declarations, + ) replaced_skill_names = set(registered_skills.get(agent_name) or []) - for tmpl in manifest.templates: - if tmpl.get("type") != "command": - continue + for tmpl in concrete_declarations: primary_name = tmpl.get("name") if not isinstance(primary_name, str): continue @@ -408,9 +558,7 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: modern_name in replaced_skill_names or legacy_name in replaced_skill_names ): - successful_skill_replacements.add( - (pack_id, primary_name) - ) + successful_skill_replacements.add((pack_id, primary_name)) raw_existing_skills = metadata.get("registered_skills") if isinstance(raw_existing_skills, list) and raw_existing_skills: # Legacy flat-list value: don't assume agent_name wrote @@ -435,7 +583,11 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: for name in registered_skills[agent_name] if name not in existing_names ] - elif is_command_backed and not ai_skills_now and merged_skills.get(agent_name): + elif ( + is_command_backed + and not ai_skills_now + and merged_skills.get(agent_name) + ): # Mirror image: toggled skills -> command for this same # agent. _get_skills_dir() no longer resolves a skills # directory once ai_skills is off, so _register_skills @@ -453,11 +605,11 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: # anything unreplaced stays tracked and on disk (#2948). stale_skill_names = merged_skills[agent_name] skill_to_primary: Dict[str, str] = {} - for tmpl in manifest.templates: - if tmpl.get("type") != "command": - continue + for tmpl in concrete_declarations: primary_name = tmpl.get("name") - if not isinstance(primary_name, str): + if not isinstance(primary_name, str) or is_regex_selector( + primary_name + ): continue modern_name, legacy_name = self._skill_names_for_command( primary_name @@ -512,9 +664,7 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: # whether the *primary*'s skill replacement actually # landed (#2948). alias_to_primary: Dict[str, str] = {} - for tmpl in manifest.templates: - if tmpl.get("type") != "command": - continue + for tmpl in concrete_declarations: primary_name = tmpl.get("name") if not isinstance(primary_name, str): continue @@ -572,8 +722,7 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: for command_name, winning_pack_id in winning_pack_by_command.items() if command_name not in project_override_commands and ( - (winning_pack_id, command_name) - in successful_skill_replacements + (winning_pack_id, command_name) in successful_skill_replacements or ( command_name in reconciled_skills and command_name in winning_source_by_command @@ -609,17 +758,14 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: merged_commands[agent_name] = remaining_stale else: merged_commands.pop(agent_name, None) - self.registry.update( - pack_id, {"registered_commands": merged_commands} - ) + self.registry.update(pack_id, {"registered_commands": merged_commands}) successfully_replaced_command_winners = { command_name for command_name, winning_pack_id in winning_pack_by_command.items() if command_name not in project_override_commands and ( - (winning_pack_id, command_name) - in successful_command_replacements + (winning_pack_id, command_name) in successful_command_replacements or ( command_name in reconciled_commands and command_name in winning_source_by_command @@ -669,9 +815,7 @@ def register_enabled_presets_for_agent(self, agent_name: str) -> None: merged_skills[agent_name] = remaining_stale else: merged_skills.pop(agent_name, None) - self.registry.update( - pack_id, {"registered_skills": merged_skills} - ) + self.registry.update(pack_id, {"registered_skills": merged_skills}) def unregister_agent_artifacts(self, agent_name: str) -> None: """Remove ``agent_name``'s tracked preset command/skill artifacts. @@ -763,9 +907,8 @@ def unregister_agent_artifacts(self, agent_name: str) -> None: ) shared_names: set[str] = set() for other_agent, other_names in registered_commands.items(): - if ( - other_agent == agent_name - or not isinstance(other_names, list) + if other_agent == agent_name or not isinstance( + other_names, list ): continue other_config = registrar.AGENT_CONFIGS.get(other_agent) @@ -779,29 +922,19 @@ def unregister_agent_artifacts(self, agent_name: str) -> None: ) if other_output == agent_output: shared_names.update( - name - for name in other_names - if isinstance(name, str) + name for name in other_names if isinstance(name, str) ) command_names_to_unregister = [ - name - for name in agent_command_names - if name not in shared_names + name for name in agent_command_names if name not in shared_names ] if command_names_to_unregister: - self._unregister_commands( - {agent_name: command_names_to_unregister} - ) + self._unregister_commands({agent_name: command_names_to_unregister}) new_registered_commands = copy.deepcopy(registered_commands) new_registered_commands.pop(agent_name, None) updates["registered_commands"] = new_registered_commands agent_skill_names = registered_skills_all.get(agent_name) or [] - if ( - agent_skill_names - or skills_migrated - or native_skills_entry_removed - ): + if agent_skill_names or skills_migrated or native_skills_entry_removed: if agent_skill_names: self._delete_agent_preset_skills( agent_name, agent_skill_names, pack_id @@ -1005,6 +1138,44 @@ def record_written(written: Dict[str, List[str]]) -> None: for cmd_name in command_names: layers = resolver.collect_all_layers(cmd_name, "command") if not layers: + # A disabled/removed sole-layer preset leaves no source to + # re-register. Clean only integrations with recorded ownership + # provenance; callers update that provenance after cleanup. + tracked: Dict[str, List[str]] = {} + tracked_skills: Dict[str, Dict[str, List[str]]] = {} + for pack_id, metadata in self.registry.list_by_priority( + include_disabled=True + ): + if not isinstance(metadata, dict): + continue + recorded = metadata.get("registered_commands", {}) + if isinstance(recorded, dict): + for agent_name, names in recorded.items(): + if isinstance(names, list) and cmd_name in names: + tracked.setdefault(agent_name, []).append(cmd_name) + skills = metadata.get("registered_skills", {}) + if isinstance(skills, dict): + for agent_name, names in skills.items(): + if not isinstance(names, list): + continue + expected = set(self._skill_names_for_command(cmd_name)) + matched = [name for name in names if name in expected] + if matched: + tracked_skills.setdefault(pack_id, {}).setdefault( + agent_name, [] + ).extend(matched) + if tracked: + registrar.unregister_commands(tracked, self.project_root) + for pack_id, per_agent_skills in tracked_skills.items(): + for agent_name, skill_names in per_agent_skills.items(): + # Verify the on-disk ownership marker against the actual + # preset ID before deleting; never infer ownership from + # the selector name alone. + self._unregister_skills( + {agent_name: skill_names}, + pack_id, + restore_from_bundled_core=False, + ) continue # If the top layer is replace, it wins entirely — lower layers @@ -1024,16 +1195,40 @@ def record_written(written: Dict[str, List[str]]) -> None: if top_path.is_relative_to(pack_dir): manifest = resolver._get_manifest(pack_dir) if manifest: - for tmpl in manifest.templates: - if tmpl.get("name") == cmd_name and tmpl.get("type") == "command": - written = self._register_for_non_skill_agents( - registrar, [tmpl], manifest.id, pack_dir, - only_agent=only_agent, extra_agents=extra_agents, + concrete = next( + ( + declaration + for declaration in self._expand_command_selectors( + resolver, + pack_dir, + [ + item + for item in manifest.templates + if item.get("type") == "command" + ], ) - record_written(written) - self._merge_pack_registered_commands(manifest.id, written) - registered = True - break + if declaration.get("name") == cmd_name + and (pack_dir / str(declaration.get("file", ""))) + == top_path + ), + None, + ) + if concrete is not None: + logical_declaration = {**concrete, "name": cmd_name} + written = self._register_for_non_skill_agents( + registrar, + [logical_declaration], + manifest.id, + pack_dir, + only_agent=only_agent, + extra_agents=extra_agents, + ) + record_written(written) + self._merge_pack_registered_commands( + manifest.id, written + ) + registered = True + break break if not registered: # Top layer is a non-preset source (extension, core, or @@ -1051,15 +1246,19 @@ def record_written(written: Dict[str, List[str]]) -> None: if ext_manifest_path.exists(): try: from ..extensions import ExtensionManifest + ext_manifest = ExtensionManifest(ext_manifest_path) # Filter to only the command being reconciled matching_cmds = [ - c for c in ext_manifest.commands + c + for c in ext_manifest.commands if c.get("name") == cmd_name ] if matching_cmds: written = registrar.register_commands_for_non_skill_agents( - matching_cmds, extension_id, ext_dir, + matching_cmds, + extension_id, + ext_dir, self.project_root, context_note=f"\n\n\n", extension_id=extension_id, @@ -1075,15 +1274,16 @@ def record_written(written: Dict[str, List[str]]) -> None: if not registered: source_id = extension_id or source written = self._register_command_from_path( - registrar, cmd_name, top_path, + registrar, + cmd_name, + top_path, source_id=source_id, - only_agent=only_agent, extra_agents=extra_agents, + only_agent=only_agent, + extra_agents=extra_agents, ) record_written(written) if extension_id: - self._merge_extension_registered_commands( - extension_id, written - ) + self._merge_extension_registered_commands(extension_id, written) else: # Composed command — resolve from full stack composed = resolver.resolve_content(cmd_name, "command") @@ -1091,6 +1291,7 @@ def record_written(written: Dict[str, List[str]]) -> None: # Composition no longer possible (e.g. base layer removed). # Unregister any stale command file from non-skill agents. import warnings + warnings.warn( f"Cannot compose command '{cmd_name}': no base layer. " f"Stale command files may remain.", @@ -1104,7 +1305,10 @@ def record_written(written: Dict[str, List[str]]) -> None: _m = resolver._get_manifest(_pd) if _m: for _t in _m.templates: - if _t.get("name") == cmd_name and _t.get("type") == "command": + if ( + _t.get("name") == cmd_name + and _t.get("type") == "command" + ): for alias in _t.get("aliases", []): if isinstance(alias, str): cmd_names_to_unregister.append(alias) @@ -1118,7 +1322,8 @@ def record_written(written: Dict[str, List[str]]) -> None: { agent: cmd_names_to_unregister for agent in registrar.AGENT_CONFIGS - if registrar.AGENT_CONFIGS[agent].get("extension") != "/SKILL.md" + if registrar.AGENT_CONFIGS[agent].get("extension") + != "/SKILL.md" and ( only_agent is None or agent == only_agent @@ -1136,17 +1341,33 @@ def record_written(written: Dict[str, List[str]]) -> None: manifest = resolver._get_manifest(pack_dir) if not manifest: continue - for tmpl in manifest.templates: - if tmpl.get("name") == cmd_name and tmpl.get("type") == "command": + for tmpl in self._expand_command_selectors( + resolver, + pack_dir, + [ + item + for item in manifest.templates + if item.get("type") == "command" + ], + ): + if tmpl.get("name") == cmd_name: + logical_declaration = {**tmpl, "name": cmd_name} composed_dir = pack_dir / ".composed" composed_dir.mkdir(parents=True, exist_ok=True) composed_file = composed_dir / f"{cmd_name}.md" composed_file.write_text(composed, encoding="utf-8") written = self._register_for_non_skill_agents( registrar, - [{**tmpl, "file": f".composed/{cmd_name}.md"}], - manifest.id, pack_dir, - only_agent=only_agent, extra_agents=extra_agents, + [ + { + **logical_declaration, + "file": f".composed/{cmd_name}.md", + } + ], + manifest.id, + pack_dir, + only_agent=only_agent, + extra_agents=extra_agents, ) record_written(written) self._merge_pack_registered_commands(manifest.id, written) @@ -1168,15 +1389,16 @@ def record_written(written: Dict[str, List[str]]) -> None: else: source_id = source written = self._register_command_from_path( - registrar, cmd_name, composed_file, + registrar, + cmd_name, + composed_file, source_id=source_id, - only_agent=only_agent, extra_agents=extra_agents, + only_agent=only_agent, + extra_agents=extra_agents, ) record_written(written) if source.startswith("extension:"): - self._merge_extension_registered_commands( - source_id, written - ) + self._merge_extension_registered_commands(source_id, written) return reconciled_commands @@ -1219,7 +1441,10 @@ def _register_command_from_path( if source_id and not source_id.startswith("preset:"): try: from ..extensions import ExtensionManifest - for ext_dir in (self.project_root / ".specify" / "extensions").iterdir(): + + for ext_dir in ( + self.project_root / ".specify" / "extensions" + ).iterdir(): if not ext_dir.is_dir(): continue if cmd_path.is_relative_to(ext_dir): @@ -1236,8 +1461,12 @@ def _register_command_from_path( except Exception: pass # best-effort alias loading return self._register_for_non_skill_agents( - registrar, [cmd_tmpl], source_id, cmd_path.parent, - only_agent=only_agent, extra_agents=extra_agents, + registrar, + [cmd_tmpl], + source_id, + cmd_path.parent, + only_agent=only_agent, + extra_agents=extra_agents, ) def _register_for_non_skill_agents( @@ -1279,6 +1508,10 @@ def _register_for_non_skill_agents( ``registered_commands`` tracking (#2948). """ return registrar.register_commands_for_non_skill_agents( - commands, source_id, source_dir, self.project_root, - only_agent=only_agent, extra_agents=extra_agents, + commands, + source_id, + source_dir, + self.project_root, + only_agent=only_agent, + extra_agents=extra_agents, ) diff --git a/src/specify_cli/presets/_manager_skills.py b/src/specify_cli/presets/_manager_skills.py index e6246232af..42282fb782 100644 --- a/src/specify_cli/presets/_manager_skills.py +++ b/src/specify_cli/presets/_manager_skills.py @@ -17,6 +17,7 @@ from ._manager_commands import _substitute_core_template from ._manifest import PresetManifest, PresetValidationError from ._resolver import PresetResolver +from ._selectors import is_regex_selector class _PresetSkillMethods: @@ -29,18 +30,20 @@ class _FilteredManifest: that aren't being reconciled. """ - def __init__(self, manifest: "PresetManifest", cmd_names: set): + def __init__(self, manifest: "PresetManifest", cmd_names: set, commands=None): self._manifest = manifest self._cmd_names = cmd_names + self._commands = commands def __getattr__(self, name: str): return getattr(self._manifest, name) @property def templates(self) -> List[Dict[str, Any]]: + if self._commands is not None: + return [t for t in self._commands if t.get("name") in self._cmd_names] return [ - t for t in self._manifest.templates - if t.get("name") in self._cmd_names + t for t in self._manifest.templates if t.get("name") in self._cmd_names ] def _merge_pack_registered_skills( @@ -82,9 +85,7 @@ def _merge_pack_registered_skills( else: existing_skills = self._normalize_registered_skills(raw_existing_skills) merged_skills = copy.deepcopy(existing_skills) - changed = ( - isinstance(raw_existing_skills, list) and bool(raw_existing_skills) - ) + changed = isinstance(raw_existing_skills, list) and bool(raw_existing_skills) for agent_name, skill_names in written.items(): if not skill_names: continue @@ -99,9 +100,7 @@ def _merge_pack_registered_skills( def _reconcile_skills( self, command_names: List[str], - extra_skills_dirs: Optional[ - Dict[Path, tuple[Optional[str], List[str]]] - ] = None, + extra_skills_dirs: Optional[Dict[Path, tuple[Optional[str], List[str]]]] = None, target_agent: Optional[str] = None, ) -> Set[str]: """Re-register skills for commands whose winning layer changed. @@ -140,7 +139,11 @@ def _reconcile_skills( if not isinstance(active_ai, str) or not active_ai: active_ai = None - # Cache registry once to avoid repeated filesystem reads + # Preserve disabled entries as cleanup provenance while resolving winners + # only from enabled presets. + all_presets_by_priority = list( + self.registry.list_by_priority(include_disabled=True) + ) presets_by_priority = list(self.registry.list_by_priority()) # Group command names by winning preset to batch _register_skills calls @@ -158,14 +161,12 @@ def _reconcile_skills( if not layers: continue - skill_name, legacy_skill_name = self._skill_names_for_command( - cmd_name - ) + skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name) candidate_skill_names = {skill_name, legacy_skill_name} # Track whether any preset previously registered this skill # (i.e., it was actively managed), so a not-yet-existing skill # dir can be re-created per affected directory below. - for _pid, meta in presets_by_priority: + for _pid, meta in all_presets_by_priority: if not isinstance(meta, dict): continue recorded = meta.get("registered_skills", []) @@ -180,9 +181,7 @@ def _reconcile_skills( recorded_names = set(recorded) else: recorded_names = set() - recorded_candidates = ( - candidate_skill_names & recorded_names - ) + recorded_candidates = candidate_skill_names & recorded_names if recorded_candidates: managed_skill_names.update(recorded_candidates) @@ -196,12 +195,14 @@ def _reconcile_skills( found_preset = True break if not found_preset: - # Winner is a non-preset source (core/extension/override). - # Track the winning layer path for skill restoration. non_preset_skills.append((skill_name, cmd_name, layers[0])) - core_ext_skills = [s for s in non_preset_skills if s[2]["source"] != "project override"] - override_skills = [s for s in non_preset_skills if s[2]["source"] == "project override"] + core_ext_skills = [ + s for s in non_preset_skills if s[2]["source"] != "project override" + ] + override_skills = [ + s for s in non_preset_skills if s[2]["source"] == "project override" + ] def apply_to_dir( skills_dir: Path, @@ -243,12 +244,13 @@ def apply_to_dir( from .. import SKILL_DESCRIPTIONS from ..agents import CommandRegistrar from ..shared_infra import _write_shared_text + registrar = CommandRegistrar() content = top_layer["path"].read_text(encoding="utf-8") fm, body = registrar.parse_frontmatter(content) short_name = cmd_name if short_name.startswith("speckit."): - short_name = short_name[len("speckit."):] + short_name = short_name[len("speckit.") :] desc = fm.get("description", "") or SKILL_DESCRIPTIONS.get( short_name.replace(".", "-"), f"Command: {short_name}", @@ -262,6 +264,7 @@ def apply_to_dir( body, registrar, selected_ai, self.project_root ) from ..integrations import get_integration + integration = get_integration(selected_ai) if selected_ai else None skill_title = self._skill_title_from_command(cmd_name) wrote_override = False @@ -281,9 +284,7 @@ def apply_to_dir( desc, f"override:{cmd_name}", ) - registrar.apply_argument_hint( - fm, fm_data, integration - ) + registrar.apply_argument_hint(fm, fm_data, integration) fm_text = dump_frontmatter(fm_data) skill_content = ( f"---\n{fm_text}\n---\n\n" @@ -292,10 +293,8 @@ def apply_to_dir( if integration is not None and hasattr( integration, "post_process_skill_content" ): - skill_content = ( - integration.post_process_skill_content( - skill_content - ) + skill_content = integration.post_process_skill_content( + skill_content ) _write_shared_text( skills_dir, @@ -303,12 +302,8 @@ def apply_to_dir( skill_content, ) wrote_override = True - if ( - wrote_override - and ( - target_agent is None - or dir_agent == target_agent - ) + if wrote_override and ( + target_agent is None or dir_agent == target_agent ): reconciled_skill_commands.add(cmd_name) except Exception: @@ -337,7 +332,22 @@ def apply_to_dir( except PresetValidationError: continue cmds_set = set(dir_cmds) - filtered_manifest = self._FilteredManifest(manifest, cmds_set) + from ._manager_commands import _PresetCommandMethods + + command_methods = _PresetCommandMethods() + command_methods.__dict__.update(self.__dict__) + concrete_declarations = command_methods._expand_command_selectors( + resolver, + pack_dir, + [ + item + for item in manifest.templates + if item.get("type") == "command" + ], + ) + filtered_manifest = self._FilteredManifest( + manifest, cmds_set, concrete_declarations + ) # Not dead code: _register_skills only *overwrites* skill # subdirectories that already exist (plus brand-new ones for # the active ai_skills agent). For a restore into a @@ -364,14 +374,14 @@ def apply_to_dir( written = self._register_skills(filtered_manifest, pack_dir) else: written = self._register_skills( - filtered_manifest, pack_dir, - target_dir=skills_dir, target_agent=dir_agent or "", + filtered_manifest, + pack_dir, + target_dir=skills_dir, + target_agent=dir_agent or "", ) if target_agent is None: written_names = { - name - for names in written.values() - for name in names + name for names in written.values() for name in names } else: written_names = set(written.get(target_agent, [])) @@ -399,9 +409,7 @@ def apply_to_dir( active_ai, is_active=True, managed_names=( - set(active_provenance[1]) - if active_provenance - else None + set(active_provenance[1]) if active_provenance else None ), ) @@ -455,6 +463,7 @@ def _get_skills_dir(self) -> Optional[Path]: resolve_active_skills_dir, ) from ..shared_infra import _ensure_safe_shared_directory + opts = load_init_options(self.project_root) if isinstance(opts, dict) and opts.get("ai") == "generic": return None @@ -462,7 +471,10 @@ def _get_skills_dir(self) -> Optional[Path]: skills_dir = resolve_active_skills_dir(self.project_root) except (ValueError, OSError) as exc: _print_cli_warning( - "resolve", "skills directory", None, exc, + "resolve", + "skills directory", + None, + exc, continuing="Continuing without skill registration.", ) return None @@ -495,7 +507,10 @@ def _get_skills_dir(self) -> Optional[Path]: ) except (ValueError, OSError) as exc: _print_cli_warning( - "resolve", "skills directory", str(agent_skills_dir), exc, + "resolve", + "skills directory", + str(agent_skills_dir), + exc, continuing="Continuing without skill registration.", ) return None @@ -506,7 +521,7 @@ def _skill_names_for_command(cmd_name: str) -> tuple[str, str]: """Return the modern and legacy skill directory names for a command.""" raw_short_name = cmd_name if raw_short_name.startswith("speckit."): - raw_short_name = raw_short_name[len("speckit."):] + raw_short_name = raw_short_name[len("speckit.") :] modern_skill_name = f"speckit-{raw_short_name.replace('.', '-')}" legacy_skill_name = f"speckit.{raw_short_name}" @@ -517,7 +532,7 @@ def _skill_title_from_command(cmd_name: str) -> str: """Return a human-friendly title for a skill command name.""" title_name = cmd_name if title_name.startswith("speckit."): - title_name = title_name[len("speckit."):] + title_name = title_name[len("speckit.") :] return title_name.replace(".", " ").replace("-", " ").title() @staticmethod @@ -610,7 +625,9 @@ def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]: "extension_id": manifest.id, "extension_dir": ext_root, } - modern_skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name) + modern_skill_name, legacy_skill_name = self._skill_names_for_command( + cmd_name + ) restore_index.setdefault(modern_skill_name, restore_info) if legacy_skill_name != modern_skill_name: restore_index.setdefault(legacy_skill_name, restore_info) @@ -624,6 +641,7 @@ def _register_skills( *, target_dir: Optional[Path] = None, target_agent: Optional[str] = None, + command_templates: Optional[List[Dict[str, Any]]] = None, ) -> Dict[str, List[str]]: """Generate SKILL.md files for preset command overrides. @@ -656,9 +674,21 @@ def _register_skills( matching the shape ``registered_commands`` already uses so the two can be tracked/restored consistently (#2948). """ - command_templates = [ + command_declarations = [ t for t in manifest.templates if t.get("type") == "command" ] + command_templates = ( + command_templates + if command_templates is not None + else self._expand_command_selectors( + PresetResolver(self.project_root), preset_dir, command_declarations + ) + ) + command_templates = [ + t + for t in command_templates + if isinstance(t.get("name"), str) and not is_regex_selector(t["name"]) + ] if not command_templates: return {} @@ -698,7 +728,9 @@ def _register_skills( # preset skills in _register_commands() because their detected agent # directory is already the skills directory. This flag is only for # command-backed agents that also mirror commands into skills. - create_missing_skills = ai_skills_enabled and agent_config.get("extension") != "/SKILL.md" + create_missing_skills = ( + ai_skills_enabled and agent_config.get("extension") != "/SKILL.md" + ) written: List[str] = [] @@ -718,7 +750,7 @@ def _register_skills( # Derive the short command name (e.g. "specify" from "speckit.specify") raw_short_name = cmd_name if raw_short_name.startswith("speckit."): - raw_short_name = raw_short_name[len("speckit."):] + raw_short_name = raw_short_name[len("speckit.") :] short_name = raw_short_name.replace(".", "-") skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name) skill_title = self._skill_title_from_command(cmd_name) @@ -729,7 +761,10 @@ def _register_skills( target_skill_names: List[str] = [] if (skills_dir / skill_name).is_dir(): target_skill_names.append(skill_name) - if legacy_skill_name != skill_name and (skills_dir / legacy_skill_name).is_dir(): + if ( + legacy_skill_name != skill_name + and (skills_dir / legacy_skill_name).is_dir() + ): target_skill_names.append(legacy_skill_name) if not target_skill_names and create_missing_skills: missing_skill_dir = skills_dir / skill_name @@ -754,9 +789,7 @@ def _register_skills( # _register_commands already warned for this command in the same # pass, so the skip is silent here to avoid a duplicate warning. effective_strategy = ( - cmd_tmpl.get("strategy") - or frontmatter.get("strategy") - or "replace" + cmd_tmpl.get("strategy") or frontmatter.get("strategy") or "replace" ) if ( effective_strategy != "replace" @@ -766,7 +799,9 @@ def _register_skills( continue if frontmatter.get("strategy") == "wrap": - body, core_frontmatter = _substitute_core_template(body, cmd_name, self.project_root, registrar) + body, core_frontmatter = _substitute_core_template( + body, cmd_name, self.project_root, registrar + ) frontmatter = dict(frontmatter) for key in ("scripts", "agent_scripts", "argument-hint"): if key not in frontmatter and key in core_frontmatter: @@ -782,7 +817,9 @@ def _register_skills( body = registrar.resolve_skill_placeholders( selected_ai, frontmatter, body, self.project_root ) - body = self._resolve_skill_command_refs(body, registrar, selected_ai, self.project_root) + body = self._resolve_skill_command_refs( + body, registrar, selected_ai, self.project_root + ) for target_skill_name in target_skill_names: skill_subdir = skills_dir / target_skill_name @@ -803,7 +840,9 @@ def _register_skills( enhanced_desc, f"preset:{manifest.id}", ) - registrar.apply_argument_hint(frontmatter, frontmatter_data, integration) + registrar.apply_argument_hint( + frontmatter, frontmatter_data, integration + ) frontmatter_text = dump_frontmatter(frontmatter_data) skill_content = ( f"---\n" @@ -812,15 +851,15 @@ def _register_skills( f"# Speckit {skill_title} Skill\n\n" f"{body}\n" ) - if integration is not None and hasattr(integration, "post_process_skill_content"): + if integration is not None and hasattr( + integration, "post_process_skill_content" + ): skill_content = integration.post_process_skill_content( skill_content ) skill_file = skill_subdir / "SKILL.md" - _write_shared_text( - skills_dir, skill_file, skill_content - ) + _write_shared_text(skills_dir, skill_file, skill_content) written.append(target_skill_name) self._merge_pack_registered_skills( manifest.id, {selected_ai: [target_skill_name]} @@ -912,7 +951,9 @@ def _infer_legacy_skill_provenance( inferred: Dict[str, List[str]] = {} matched_names: set = set() for resolved_dir, agents in dir_to_agents.items(): - canonical_agent = fallback_agent if fallback_agent in agents else sorted(agents)[0] + canonical_agent = ( + fallback_agent if fallback_agent in agents else sorted(agents)[0] + ) for name in safe_skill_names: skill_subdir = resolved_dir / name if not self._validate_skill_subdir( @@ -1000,8 +1041,10 @@ def _safe_skills_dir_for_agent(self, agent_name: str) -> Optional[Path]: return None try: _ensure_safe_shared_directory( - validation_root, skills_dir, - create=False, context="preset skills directory", + validation_root, + skills_dir, + create=False, + context="preset skills directory", ) except (ValueError, OSError): return None @@ -1071,13 +1114,13 @@ def _validate_skill_subdir( try: if create: _ensure_safe_shared_directory( - validation_root, skill_subdir, - create=True, context="preset skill directory", + validation_root, + skill_subdir, + create=True, + context="preset skill directory", ) else: - _validate_safe_shared_directory( - validation_root, skill_subdir - ) + _validate_safe_shared_directory(validation_root, skill_subdir) except (ValueError, OSError): return False return True @@ -1215,11 +1258,7 @@ def _unregister_skills( additional_owned_sources=additional_owned_sources, restore_from_bundled_core=restore_from_bundled_core, ) - return ( - {skills_dir: (selected_ai, mutated_names)} - if mutated_names - else {} - ) + return {skills_dir: (selected_ai, mutated_names)} if mutated_names else {} def _delete_agent_preset_skills( self, agent_name: str, skill_names: List[str], pack_id: str @@ -1240,16 +1279,9 @@ def _delete_agent_preset_skills( if manifest is not None: for template in manifest.templates: command_name = template.get("name") - if ( - template.get("type") == "command" - and isinstance(command_name, str) - ): - for skill_name in self._skill_names_for_command( - command_name - ): - override_sources[skill_name] = ( - f"override:{command_name}" - ) + if template.get("type") == "command" and isinstance(command_name, str): + for skill_name in self._skill_names_for_command(command_name): + override_sources[skill_name] = f"override:{command_name}" for skill_name in skill_names: if not self._is_safe_registry_skill_name(skill_name): continue @@ -1267,11 +1299,7 @@ def _delete_agent_preset_skills( continue frontmatter, _ = registrar.parse_frontmatter(content) metadata = frontmatter.get("metadata") - source = ( - metadata.get("source") - if isinstance(metadata, dict) - else None - ) + source = metadata.get("source") if isinstance(metadata, dict) else None owned_sources = {marker} override_source = override_sources.get(skill_name) if override_source: @@ -1335,7 +1363,9 @@ def _unregister_skills_in_dir( # Locate core command templates from the project's installed templates core_templates_dir = self.project_root / ".specify" / "templates" / "commands" registrar = CommandRegistrar() - integration = get_integration(selected_ai) if isinstance(selected_ai, str) else None + integration = ( + get_integration(selected_ai) if isinstance(selected_ai, str) else None + ) extension_restore_index = self._build_extension_skill_restore_index() mutated_names: List[str] = [] @@ -1357,9 +1387,9 @@ def _unregister_skills_in_dir( # Derive command name from skill name (speckit-specify -> specify) short_name = skill_name if short_name.startswith("speckit-"): - short_name = short_name[len("speckit-"):] + short_name = short_name[len("speckit-") :] elif short_name.startswith("speckit."): - short_name = short_name[len("speckit."):] + short_name = short_name[len("speckit.") :] skill_subdir = skills_dir / skill_name skill_file = skill_subdir / "SKILL.md" @@ -1390,9 +1420,7 @@ def _unregister_skills_in_dir( ) owned_sources = {f"preset:{pack_id}"} if additional_owned_sources: - additional_source = additional_owned_sources.get( - skill_name - ) + additional_source = additional_owned_sources.get(skill_name) if additional_source: owned_sources.add(additional_source) if current_source not in owned_sources: @@ -1424,7 +1452,9 @@ def _unregister_skills_in_dir( if _core_pack is not None: core_file = _core_pack / "commands" / f"{short_name}.md" else: - core_file = _repo_root() / "templates" / "commands" / f"{short_name}.md" + core_file = ( + _repo_root() / "templates" / "commands" / f"{short_name}.md" + ) if not core_file.exists(): core_file = None @@ -1464,7 +1494,9 @@ def _unregister_skills_in_dir( enhanced_desc, f"templates/commands/{short_name}.md", ) - registrar.apply_argument_hint(frontmatter, frontmatter_data, integration) + registrar.apply_argument_hint( + frontmatter, frontmatter_data, integration + ) frontmatter_text = dump_frontmatter(frontmatter_data) skill_title = self._skill_title_from_command(short_name) skill_content = ( @@ -1474,7 +1506,9 @@ def _unregister_skills_in_dir( f"# Speckit {skill_title} Skill\n\n" f"{body}\n" ) - if integration is not None and hasattr(integration, "post_process_skill_content"): + if integration is not None and hasattr( + integration, "post_process_skill_content" + ): skill_content = integration.post_process_skill_content( skill_content ) @@ -1487,7 +1521,9 @@ def _unregister_skills_in_dir( # unreadable extension source leaves the skill in place # instead of crashing or being deleted. try: - content = extension_restore["source_file"].read_text(encoding="utf-8") + content = extension_restore["source_file"].read_text( + encoding="utf-8" + ) except (OSError, UnicodeDecodeError) as exc: self._warn_unrestored_skill( skill_name, extension_restore["source_file"], exc @@ -1519,20 +1555,22 @@ def _unregister_skills_in_dir( frontmatter_data = registrar.build_skill_frontmatter( selected_ai if isinstance(selected_ai, str) else "", skill_name, - frontmatter.get("description", f"Extension command: {command_name}"), + frontmatter.get( + "description", f"Extension command: {command_name}" + ), extension_restore["source"], author=extension_restore.get("author", "github-spec-kit"), ) - registrar.apply_argument_hint(frontmatter, frontmatter_data, integration) + registrar.apply_argument_hint( + frontmatter, frontmatter_data, integration + ) frontmatter_text = dump_frontmatter(frontmatter_data) skill_content = ( - f"---\n" - f"{frontmatter_text}\n" - f"---\n\n" - f"# {title_name} Skill\n\n" - f"{body}\n" + f"---\n{frontmatter_text}\n---\n\n# {title_name} Skill\n\n{body}\n" ) - if integration is not None and hasattr(integration, "post_process_skill_content"): + if integration is not None and hasattr( + integration, "post_process_skill_content" + ): skill_content = integration.post_process_skill_content( skill_content ) diff --git a/src/specify_cli/presets/_manifest.py b/src/specify_cli/presets/_manifest.py index b5a7ff03c3..90878ad2fe 100644 --- a/src/specify_cli/presets/_manifest.py +++ b/src/specify_cli/presets/_manifest.py @@ -13,16 +13,19 @@ class PresetError(Exception): """Base exception for preset-related errors.""" + pass class PresetValidationError(PresetError): """Raised when preset manifest validation fails.""" + pass class PresetCompatibilityError(PresetError): """Raised when preset is incompatible with current environment.""" + pass @@ -54,7 +57,7 @@ def __init__(self, manifest_path: Path): def _load_yaml(self, path: Path) -> dict: """Load YAML file safely.""" try: - with open(path, 'r', encoding='utf-8') as f: + with open(path, "r", encoding="utf-8") as f: data = yaml.safe_load(f) except yaml.YAMLError as e: raise PresetValidationError(f"Invalid YAML in {path}: {e}") @@ -90,9 +93,7 @@ def _validate(self): for section in ("preset", "requires", "provides"): if not isinstance(self.data[section], dict): - raise PresetValidationError( - f"Invalid {section}: expected a mapping" - ) + raise PresetValidationError(f"Invalid {section}: expected a mapping") # Validate preset metadata pack = self.data["preset"] @@ -117,7 +118,7 @@ def _validate(self): ) # Validate pack ID format - if not re.match(r'^[a-z0-9-]+$', pack["id"]): + if not re.match(r"^[a-z0-9-]+$", pack["id"]): raise PresetValidationError( f"Invalid preset ID '{pack['id']}': " "must be lowercase alphanumeric with hyphens only" @@ -164,9 +165,7 @@ def _validate(self): # Validate provides section provides = self.data["provides"] if "templates" not in provides: - raise PresetValidationError( - "Preset must provide at least one template" - ) + raise PresetValidationError("Preset must provide at least one template") # Validate templates. Guard the container and each entry's shape so a # malformed third-party preset.yml (e.g. ``templates: 5`` or @@ -182,13 +181,9 @@ def _validate(self): # latter, since that genuinely is a list with no templates. templates = provides["templates"] if not isinstance(templates, list): - raise PresetValidationError( - "Invalid provides.templates: expected a list" - ) + raise PresetValidationError("Invalid provides.templates: expected a list") if not templates: - raise PresetValidationError( - "Preset must provide at least one template" - ) + raise PresetValidationError("Preset must provide at least one template") seen_name_types: set[tuple[str, str]] = set() for tmpl in templates: if not isinstance(tmpl, dict): @@ -264,20 +259,32 @@ def _validate(self): f"scripts only support {sorted(VALID_SCRIPT_STRATEGIES)}" ) - # Validate template name format - if tmpl["type"] == "command": - # Commands use dot notation (e.g. speckit.specify) - if not re.match(r'^[a-z0-9.-]+$', tmpl["name"]): + # Regex selectors are validated here, before installation or resolution. + # The logical resource name grammar is validated separately below. + from ._selectors import compile_name_selector, is_regex_selector + + if is_regex_selector(tmpl["name"]): + try: + compile_name_selector(tmpl["name"]) + except re.error as exc: raise PresetValidationError( - f"Invalid command name '{tmpl['name']}': " - "must be lowercase alphanumeric with hyphens and dots only" - ) + f"Invalid regex selector in template name {tmpl['name']!r}: {exc}" + ) from exc else: - if not re.match(r'^[a-z0-9-]+$', tmpl["name"]): - raise PresetValidationError( - f"Invalid template name '{tmpl['name']}': " - "must be lowercase alphanumeric with hyphens only" - ) + # Validate template name format + if tmpl["type"] == "command": + # Commands use dot notation (e.g. speckit.specify) + if not re.match(r"^[a-z0-9.-]+$", tmpl["name"]): + raise PresetValidationError( + f"Invalid command name '{tmpl['name']}': " + "must be lowercase alphanumeric with hyphens and dots only" + ) + else: + if not re.match(r"^[a-z0-9-]+$", tmpl["name"]): + raise PresetValidationError( + f"Invalid template name '{tmpl['name']}': " + "must be lowercase alphanumeric with hyphens only" + ) @property def id(self) -> str: @@ -355,7 +362,7 @@ def _validate_requires_extensions(declared: Any) -> None: # otherwise validate here while PresetResolver._is_safe_registry_id # (which uses fullmatch) rejects it, and the newline would land in # a suggested command. - if not re.fullmatch(r'[a-z0-9-]+', extension_id): + if not re.fullmatch(r"[a-z0-9-]+", extension_id): raise PresetValidationError( f"Invalid {label}.id {extension_id!r}: " "must be lowercase alphanumeric with hyphens only" @@ -432,7 +439,7 @@ def tags(self) -> List[str]: def get_hash(self) -> str: """Calculate SHA256 hash of manifest file.""" h = hashlib.sha256() - with open(self.path, 'rb') as f: + with open(self.path, "rb") as f: for chunk in iter(lambda: f.read(8192), b""): h.update(chunk) return f"sha256:{h.hexdigest()}" diff --git a/src/specify_cli/presets/_resolver.py b/src/specify_cli/presets/_resolver.py index ea1c4e03f9..b931b2f01f 100644 --- a/src/specify_cli/presets/_resolver.py +++ b/src/specify_cli/presets/_resolver.py @@ -10,6 +10,7 @@ from ..extensions import ExtensionRegistry, normalize_priority from ._manifest import VALID_PRESET_STRATEGIES, PresetManifest, PresetValidationError from ._registry import PresetRegistry +from ._selectors import is_regex_selector, selector_matches class PresetResolver: @@ -84,7 +85,13 @@ def _manifest_declared_template( if not manifest: return None, None for tmpl in manifest.templates: - if tmpl.get("name") == template_name and tmpl.get("type") == template_type: + declared_name = tmpl.get("name") + if ( + isinstance(declared_name, str) + and not is_regex_selector(declared_name) + and declared_name == template_name + and tmpl.get("type") == template_type + ): file_path = tmpl.get("file") if file_path: manifest_candidate = pack_dir / file_path @@ -97,22 +104,7 @@ def _manifest_declared_template( def _extension_manifest_declared_template( self, ext_dir: Path, template_name: str, template_type: str ) -> tuple[dict | None, Path | None]: - """Resolve an extension's manifest-declared command/template/script entry and usable file. - - Mirrors ``_manifest_declared_template`` (for presets): returns ``(entry, candidate)`` - where ``entry`` is the matching ``provides.`` mapping, or ``None`` if the - extension has no (valid) manifest or doesn't declare this ``(name, type)``. - ``candidate`` is the declared ``file:`` resolved under ``ext_dir`` IFF it is a - regular file that stays within ``ext_dir`` (guards against path traversal via a - malformed manifest, mirroring ``resolve_extension_command_via_manifest``); - ``None`` otherwise. - - The manifest is authoritative: when ``entry`` is not ``None`` but ``candidate`` is - ``None``, callers must NOT fall back to convention-based lookup — that would mask - a typo or pick up an undeclared file. Shared by ``resolve()`` and - ``collect_all_layers()`` so their manifest-first resolution cannot silently - diverge (the divergence flagged in review on #4012). - """ + """Resolve an extension manifest declaration with conventional fallback.""" if template_type not in ("command", "template", "script"): return None, None ext_manifest_path = ext_dir / "extension.yml" @@ -125,14 +117,17 @@ def _extension_manifest_declared_template( ext_manifest = ExtensionManifest(ext_manifest_path) except (ExtValidationError, yaml.YAMLError, OSError, TypeError, AttributeError): return None, None - if template_type == "command": - entries = ext_manifest.commands - elif template_type == "template": - entries = ext_manifest.templates - else: - entries = ext_manifest.scripts + entries = { + "command": ext_manifest.commands, + "template": ext_manifest.templates, + "script": ext_manifest.scripts, + }[template_type] for entry in entries: - if entry.get("name") != template_name: + name = entry.get("name") + if name != template_name and not ( + template_type == "command" + and name == template_name.removeprefix("speckit.") + ): continue file_rel = entry.get("file") if not file_rel: @@ -142,17 +137,39 @@ def _extension_manifest_declared_template( return entry, None candidate = ext_dir / rel_path try: - # Resolve only for the containment check, not for the - # returned path -- resolving the returned path would follow - # symlinks in ext_dir's ancestors (e.g. a symlinked tmp dir - # on macOS) and diverge from the unresolved paths convention - # lookup returns for the same directory. - candidate.resolve().relative_to(ext_dir.resolve()) # raises ValueError if outside + candidate.resolve().relative_to(ext_dir.resolve()) except (OSError, ValueError): return entry, None return entry, (candidate if candidate.is_file() else None) return None, None + def _find_unregistered_extension_command(self, template_name: str) -> Path | None: + """Find the legacy extension filename only for unregistered extensions.""" + extension_template_name = template_name.removeprefix("speckit.") + namespace = extension_template_name.split(".", 1)[0] + ext_dir = self.extensions_dir / namespace + registry = ExtensionRegistry(self.extensions_dir) + if namespace in registry.keys(): + metadata = registry.get(namespace) + if metadata is None or not metadata.get("enabled", True): + return None + manifest_path = ext_dir / "extension.yml" + if manifest_path.is_file(): + entry, candidate = self._extension_manifest_declared_template( + ext_dir, template_name, "command" + ) + if entry is not None: + return candidate + fallback_name = extension_template_name + alternate_candidates = ( + ext_dir / "commands" / f"{fallback_name}.md", + ext_dir / "templates" / "commands" / f"{fallback_name}.md", + ) + for candidate in alternate_candidates: + if candidate.is_file(): + return candidate + return None + def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]: """Build unified list of registered and unregistered extensions sorted by priority. @@ -193,7 +210,9 @@ def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]: # Skip disabled extensions if not metadata.get("enabled", True): continue - priority = normalize_priority(metadata.get("priority") if metadata else None) + priority = normalize_priority( + metadata.get("priority") if metadata else None + ) all_extensions.append((priority, ext_id, metadata)) # Add unregistered directories with implicit priority=10 @@ -217,7 +236,7 @@ def _core_stem(template_name: str) -> Optional[str]: or ``None`` otherwise. """ if template_name.startswith("speckit."): - return template_name[len("speckit."):] + return template_name[len("speckit.") :] return None def resolve( @@ -254,7 +273,6 @@ def resolve( if template_type == "script": ext = ".sh" # scripts use .sh; callers can also check .ps1 - # Priority 1: Project-local overrides if template_type == "script": override = self.overrides_dir / "scripts" / f"{template_name}{ext}" else: @@ -278,6 +296,10 @@ def resolve( entry, manifest_candidate = self._manifest_declared_template( pack_dir, template_name, template_type ) + if entry is None: + entry, manifest_candidate = self._regex_preset_declaration( + pack_dir, template_name, template_type + ) if manifest_candidate is not None: return manifest_candidate if entry is not None: @@ -318,7 +340,7 @@ def resolve( if candidate.exists(): return candidate - # Priority 4: Core templates + # Priority 3: Project templates if template_type == "template": core = self.templates_dir / f"{template_name}.md" if core.exists(): @@ -346,6 +368,7 @@ def resolve( _locate_core_pack, _repo_root, ) + _core_pack = _locate_core_pack() if _core_pack is not None: # Wheel install path @@ -504,6 +527,151 @@ def resolve_with_source( return {"path": resolved_str, "source": "core"} + def _preset_declarations_for_resource( + self, pack_dir: Path, template_name: str, template_type: str + ) -> list[tuple[dict, Path | None]]: + """Collect exact and matching regex declarations in manifest order.""" + if template_type not in {"template", "script", "command"}: + return [] + manifest = self._get_manifest(pack_dir) + if manifest is None: + return [] + presets = self._get_all_presets_by_priority() + pack_ids = [pack_id for pack_id, _ in presets] + try: + current_index = pack_ids.index(pack_dir.name) + except ValueError: + return [] + has_lower_resource = any( + self._preset_has_concrete_resource( + self.presets_dir / lower_id, template_name, template_type + ) + for lower_id, _metadata in presets[current_index + 1 :] + ) + if not has_lower_resource: + has_lower_resource = any( + self._extension_has_concrete_resource( + self.extensions_dir / ext_id, template_name, template_type + ) + for _priority, ext_id, _meta in self._get_all_extensions_by_priority() + ) + if not has_lower_resource: + has_lower_resource = self._core_has_concrete_resource( + template_name, template_type + ) + matches = [] + for declaration in manifest.templates: + name = declaration.get("name") + if declaration.get("type") != template_type or not isinstance(name, str): + continue + exact = not is_regex_selector(name) and name == template_name + regex = ( + has_lower_resource + and is_regex_selector(name) + and selector_matches(name, template_name) + ) + if exact or regex: + file_value = declaration.get("file") + path = pack_dir / file_value if isinstance(file_value, str) else None + matches.append( + (declaration, path if path is not None and path.is_file() else None) + ) + return matches + + def _regex_preset_declarations( + self, pack_dir: Path, template_name: str, template_type: str + ) -> list[tuple[dict, Path | None]]: + matches = self._preset_declarations_for_resource( + pack_dir, template_name, template_type + ) + return [ + item for item in matches if is_regex_selector(str(item[0].get("name", ""))) + ] + + def _regex_preset_declaration( + self, pack_dir: Path, template_name: str, template_type: str + ) -> tuple[dict | None, Path | None]: + matches = self._regex_preset_declarations( + pack_dir, template_name, template_type + ) + return matches[0] if matches else (None, None) + + def _has_concrete_resource( + self, + base_dir: Path, + name: str, + template_type: str, + *, + is_extension: bool = False, + ) -> bool: + """Check concrete-resource existence using the same lookups as resolve().""" + if is_extension: + entry, candidate = self._extension_manifest_declared_template( + base_dir, name, template_type + ) + else: + entry, candidate = self._manifest_declared_template( + base_dir, name, template_type + ) + if candidate is not None: + return True + if entry is not None: + return False + return self._conventional_resource(base_dir, name, template_type) is not None + + def _preset_has_concrete_resource( + self, base_dir: Path, name: str, template_type: str + ) -> bool: + return self._has_concrete_resource(base_dir, name, template_type) + + def _extension_has_concrete_resource( + self, base_dir: Path, name: str, template_type: str + ) -> bool: + return self._has_concrete_resource( + base_dir, name, template_type, is_extension=True + ) + + def _core_has_concrete_resource(self, name: str, template_type: str) -> bool: + return self._core_resource(name, template_type) is not None + + def _conventional_resource( + self, base_dir: Path, name: str, template_type: str + ) -> Optional[Path]: + subdirs = { + "template": ("templates", ""), + "command": ("commands",), + "script": ("scripts",), + }.get(template_type, ("",)) + extension = ".sh" if template_type == "script" else ".md" + for subdir in subdirs: + candidate = ( + base_dir / subdir / f"{name}{extension}" + if subdir + else base_dir / f"{name}{extension}" + ) + if candidate.is_file(): + return candidate + return None + + def _core_resource(self, name: str, template_type: str) -> Optional[Path]: + extension = ".sh" if template_type == "script" else ".md" + if template_type == "template": + candidates = [self.templates_dir / f"{name}{extension}"] + elif template_type == "command": + candidates = [self.templates_dir / "commands" / f"{name}{extension}"] + stem = self._core_stem(name) + if stem: + candidates.append( + self.templates_dir / "commands" / f"{stem}{extension}" + ) + else: + candidates = [self.templates_dir / "scripts" / f"{name}{extension}"] + for candidate in candidates: + if candidate.is_file(): + return candidate + bundled = self._find_bundled_core(name, template_type, extension) + return bundled if bundled is not None and bundled.is_file() else None + def collect_all_layers( self, template_name: str, @@ -546,73 +714,83 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: return candidate return None - # Priority 1: Project-local overrides (always "replace" strategy) if template_type == "script": override = self.overrides_dir / "scripts" / f"{template_name}{ext}" else: override = self.overrides_dir / f"{template_name}{ext}" if override.exists(): - layers.append({ - "path": override, - "source": "project override", - "strategy": "replace", - }) + layers.append( + { + "path": override, + "source": "project override", + "strategy": "replace", + } + ) # Priority 2: Installed presets (sorted by priority — lower number = higher precedence) if self.presets_dir.exists(): for pack_id, metadata in self._get_all_presets_by_priority(): pack_dir = self.presets_dir / pack_id - # Read strategy and manifest file path from preset manifest - strategy = "replace" - manifest_has_strategy = False - entry, manifest_candidate = self._manifest_declared_template( + preset_declarations = self._preset_declarations_for_resource( pack_dir, template_name, template_type ) - if entry is not None: - strategy = entry.get("strategy", "replace") - manifest_has_strategy = "strategy" in entry - # Use the manifest's declared file when it's a usable regular file; - # only fall back to convention-based lookup when the manifest - # doesn't list this template at all, so preset.yml stays - # authoritative (a declared-but-unusable file skips convention — - # parity with resolve()). - candidate = None - if manifest_candidate is not None: - candidate = manifest_candidate - elif entry is None: - candidate = _find_in_subdirs(pack_dir) - if candidate: - # Legacy fallback: if manifest doesn't explicitly declare a - # strategy, check the command file's frontmatter for any valid - # strategy. Skip when the manifest entry includes strategy key - # (even if it's "replace") to avoid overriding explicit declarations. - if not manifest_has_strategy and strategy == "replace" and template_type == "command": + if not preset_declarations: + entry, candidate = self._manifest_declared_template( + pack_dir, template_name, template_type + ) + if entry is None: + candidate = _find_in_subdirs(pack_dir) + preset_declarations = [(entry, candidate)] + for layer_entry, candidate in preset_declarations: + strategy = "replace" + manifest_has_strategy = False + if layer_entry is not None: + strategy = layer_entry.get("strategy", "replace") + manifest_has_strategy = "strategy" in layer_entry + if candidate is None: + continue + if ( + not manifest_has_strategy + and strategy == "replace" + and template_type == "command" + ): try: cmd_content = candidate.read_text(encoding="utf-8") lines = cmd_content.splitlines(keepends=True) if lines and lines[0].rstrip("\r\n") == "---": - fence_end = -1 - for fi, fline in enumerate(lines[1:], start=1): - if fline.rstrip("\r\n") == "---": - fence_end = fi - break + fence_end = next( + ( + i + for i, line in enumerate(lines[1:], 1) + if line.rstrip("\r\n") == "---" + ), + -1, + ) if fence_end > 0: - fm_text = "".join(lines[1:fence_end]) - fm_data = yaml.safe_load(fm_text) - if isinstance(fm_data, dict): - fm_strategy = fm_data.get("strategy") - if isinstance(fm_strategy, str) and fm_strategy.lower() in VALID_PRESET_STRATEGIES: - strategy = fm_strategy.lower() + fm_data = yaml.safe_load( + "".join(lines[1:fence_end]) + ) + fm_strategy = ( + fm_data.get("strategy") + if isinstance(fm_data, dict) + else None + ) + if ( + isinstance(fm_strategy, str) + and fm_strategy.lower() + in VALID_PRESET_STRATEGIES + ): + strategy = fm_strategy.lower() except (UnicodeDecodeError, yaml.YAMLError, OSError): - # Best-effort legacy frontmatter parsing: keep default - # strategy ("replace") when content is unreadable/invalid. pass version = metadata.get("version", "?") if metadata else "?" - layers.append({ - "path": candidate, - "source": f"{pack_id} v{version}", - "strategy": strategy, - }) + layers.append( + { + "path": candidate, + "source": f"{pack_id} v{version}", + "strategy": strategy, + } + ) # Priority 3: Extension-provided templates (always "replace") for _priority, ext_id, ext_meta in self._get_all_extensions_by_priority(): @@ -628,19 +806,50 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: ) if entry is None: candidate = _find_in_subdirs(ext_dir) + if candidate is None and ext_meta is None: + candidate = self._find_unregistered_extension_command(template_name) if candidate: if ext_meta: version = ext_meta.get("version", "?") source = f"extension:{ext_id} v{version}" else: source = f"extension:{ext_id} (unregistered)" - layers.append({ - "path": candidate, - "source": source, - "strategy": "replace", - "extension_id": ext_id, - "extension_dir": ext_dir, - }) + layers.append( + { + "path": candidate, + "source": source, + "strategy": "replace", + "extension_id": ext_id, + "extension_dir": ext_dir, + } + ) + + if template_type == "command": + extension_template_name = template_name.removeprefix("speckit.") + namespace = extension_template_name.split(".", 1)[0] + ext_dir = self.extensions_dir / namespace + extension_registry = ExtensionRegistry(self.extensions_dir) + extension_metadata = extension_registry.get(namespace) + namespace_is_registered = namespace in extension_registry.keys() + if not namespace_is_registered or ( + extension_metadata is not None + and extension_metadata.get("enabled", True) + ): + candidate = self._find_unregistered_extension_command(template_name) + if candidate is not None: + layers.append( + { + "path": candidate, + "source": ( + f"extension:{namespace} v{extension_metadata.get('version', '?')}" + if extension_metadata + else f"extension:{namespace} (unregistered)" + ), + "strategy": "replace", + "extension_id": namespace, + "extension_dir": ext_dir, + } + ) # Priority 4: Core templates (always "replace") core = None @@ -664,21 +873,25 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: if c.exists(): core = c if core: - layers.append({ - "path": core, - "source": "core", - "strategy": "replace", - }) + layers.append( + { + "path": core, + "source": "core", + "strategy": "replace", + } + ) else: # Priority 5: Bundled core_pack (wheel install) or repo-root # templates (source-checkout), matching resolve()'s tier-5 fallback. bundled = self._find_bundled_core(template_name, template_type, ext) if bundled: - layers.append({ - "path": bundled, - "source": "core (bundled)", - "strategy": "replace", - }) + layers.append( + { + "path": bundled, + "source": "core (bundled)", + "strategy": "replace", + } + ) return layers @@ -850,8 +1063,8 @@ def _split_frontmatter(text: str) -> tuple: if fence_end == -1: return None, text - fm_block = "".join(lines[:fence_end + 1]).rstrip("\r\n") - body = "".join(lines[fence_end + 1:]) + fm_block = "".join(lines[: fence_end + 1]).rstrip("\r\n") + body = "".join(lines[fence_end + 1 :]) return fm_block, body if is_command: @@ -907,6 +1120,7 @@ def _split_frontmatter(text: str) -> tuple: # inheriting scripts/agent_scripts from the base if missing # and stripping the strategy key (internal-only, not for agent output). if is_command and top_frontmatter_text: + def _parse_fm_yaml(fm_block: str) -> dict: """Parse YAML from a frontmatter block (with --- fences).""" lines = fm_block.splitlines() @@ -934,11 +1148,7 @@ def _parse_fm_yaml(fm_block: str) -> dict: top_fm.pop("strategy", None) if top_fm: - top_frontmatter_text = ( - "---\n" - + dump_frontmatter(top_fm) - + "\n---" - ) + top_frontmatter_text = "---\n" + dump_frontmatter(top_fm) + "\n---" else: # Empty frontmatter — omit rather than emitting {} top_frontmatter_text = None diff --git a/src/specify_cli/presets/_selectors.py b/src/specify_cli/presets/_selectors.py new file mode 100644 index 0000000000..f7ab978d2d --- /dev/null +++ b/src/specify_cli/presets/_selectors.py @@ -0,0 +1,30 @@ +"""Thin helpers for exact preset names and ``regex:`` selectors.""" + +from __future__ import annotations + +import re +from typing import Pattern + +REGEX_PREFIX = "regex:" + + +def is_regex_selector(name: str) -> bool: + """Return whether a manifest name opts into regex selector semantics.""" + return name.startswith(REGEX_PREFIX) + + +def compile_name_selector(name: str) -> Pattern[str] | None: + """Compile a regex selector, or return None for an exact-name declaration.""" + if not is_regex_selector(name): + return None + return re.compile(name[len(REGEX_PREFIX) :]) + + +def selector_matches(name: str, concrete_name: str) -> bool: + """Full-match a concrete logical name; exact declarations stay exact.""" + compiled = compile_name_selector(name) + return ( + compiled.fullmatch(concrete_name) is not None + if compiled + else name == concrete_name + ) diff --git a/src/specify_cli/presets/command_disable.py b/src/specify_cli/presets/command_disable.py index 85c0231abd..3f34e828cf 100644 --- a/src/specify_cli/presets/command_disable.py +++ b/src/specify_cli/presets/command_disable.py @@ -36,15 +36,111 @@ def preset_disable( console.print(f"[yellow]Preset '{preset_id}' is already disabled[/yellow]") raise typer.Exit(0) - # Disable the preset + from ._resolver import PresetResolver + + resolver = PresetResolver(project_root) + affected = manager._collect_selector_command_names(resolver) + manifest = resolver._get_manifest(manager.presets_dir / preset_id) + declarations = [ + item + for item in (manifest.templates if manifest is not None else []) + if item.get("type") == "command" + ] + names = { + item["name"] + for item in manager._expand_command_selectors( + resolver, manager.presets_dir / preset_id, declarations + ) + if isinstance(item.get("name"), str) + } + names.update(affected) + original_commands = metadata.get("registered_commands") or {} + original_skills = metadata.get("registered_skills") or {} + if names: + manager._reconcile_composed_commands(sorted(names)) + manager._reconcile_skills(sorted(names)) + if not all( + hasattr(manager, name) + for name in ( + "_collect_selector_command_names", + "_expand_command_selectors", + "_skill_names_for_command", + ) + ): + manager.registry.update(preset_id, {"enabled": False}) + manager.reconcile_constitution( + f"Failed to reconcile constitution after disabling preset {preset_id}" + ) + console.print(f"[green]✓[/green] Preset '{preset_id}' disabled") + console.print("\nTemplates from this preset will be skipped during resolution.") + console.print(f"To re-enable: specify preset enable {preset_id}") + return + if isinstance(original_commands, dict): + names.update( + name + for values in original_commands.values() + if isinstance(values, list) + for name in values + if isinstance(name, str) + ) manager.registry.update(preset_id, {"enabled": False}) + if names: + try: + manager._reconcile_composed_commands(sorted(names)) + manager._reconcile_skills(sorted(names)) + except Exception as exc: + import warnings + + warnings.warn( + f"Could not clean preset artifacts after disabling {preset_id}: {exc}. " + "Tracked artifact provenance was preserved for retry.", + stacklevel=2, + ) + console.print( + f"[yellow]Preset '{preset_id}' disabled; artifact cleanup failed. " + "Tracked files remain recorded for retry.[/yellow]" + ) + return + concrete_skills = { + skill_name + for command_name in names + for skill_name in manager._skill_names_for_command(command_name) + } + commands = original_commands + if isinstance(commands, dict): + updated_commands = { + agent: [name for name in values if name not in names] + for agent, values in commands.items() + if isinstance(values, list) + } + updated_commands = { + agent: values for agent, values in updated_commands.items() if values + } + else: + updated_commands = commands + skills = original_skills + if isinstance(skills, dict): + updated_skills = { + agent: [name for name in values if name not in concrete_skills] + for agent, values in skills.items() + if isinstance(values, list) + } + updated_skills = { + agent: values for agent, values in updated_skills.items() if values + } + else: + updated_skills = skills + manager.registry.update( + preset_id, + { + "registered_commands": updated_commands, + "registered_skills": updated_skills, + }, + ) manager.reconcile_constitution( f"Failed to reconcile constitution after disabling preset {preset_id}" ) console.print(f"[green]✓[/green] Preset '{preset_id}' disabled") console.print("\nTemplates from this preset will be skipped during resolution.") - console.print( - "[dim]Note: Previously registered commands/skills remain active until preset removal.[/dim]" - ) console.print(f"To re-enable: specify preset enable {preset_id}") diff --git a/src/specify_cli/presets/command_enable.py b/src/specify_cli/presets/command_enable.py index c3cd3ba5ff..98de384f47 100644 --- a/src/specify_cli/presets/command_enable.py +++ b/src/specify_cli/presets/command_enable.py @@ -36,8 +36,46 @@ def preset_enable( console.print(f"[yellow]Preset '{preset_id}' is already enabled[/yellow]") raise typer.Exit(0) - # Enable the preset + # Capture selector matches while the preset is disabled, then enable it and + # reconcile the newly active resolution stack. + from ._resolver import PresetResolver + + resolver = PresetResolver(project_root) + preset_dir = manager.presets_dir / preset_id + manifest = resolver._get_manifest(preset_dir) + declarations = [ + item + for item in (manifest.templates if manifest is not None else []) + if item.get("type") == "command" + ] + names = { + item["name"] + for item in manager._expand_command_selectors( + resolver, preset_dir, declarations + ) + if isinstance(item.get("name"), str) + } manager.registry.update(preset_id, {"enabled": True}) + from .. import load_init_options + + options = load_init_options(project_root) + active_agent = options.get("ai") if isinstance(options, dict) else None + if isinstance(active_agent, str) and active_agent: + manager.register_enabled_presets_for_agent(active_agent) + if names: + names.update( + manager._collect_selector_command_names(PresetResolver(project_root)) + ) + try: + manager._reconcile_composed_commands(sorted(names)) + manager._reconcile_skills(sorted(names)) + except Exception as exc: + import warnings + + warnings.warn( + f"Could not reconcile preset commands after enabling {preset_id}: {exc}", + stacklevel=2, + ) manager.reconcile_constitution( f"Failed to reconcile constitution after enabling preset {preset_id}" ) diff --git a/src/specify_cli/presets/command_info.py b/src/specify_cli/presets/command_info.py index a3920a6605..269bbe8c24 100644 --- a/src/specify_cli/presets/command_info.py +++ b/src/specify_cli/presets/command_info.py @@ -9,6 +9,101 @@ from ._commands import preset_app +def _diagnostic_selector_matches(resolver, preset_dir, selector, resource_type): + import os + from ._selectors import is_regex_selector, selector_matches + + ordered_presets = resolver._get_all_presets_by_priority() + try: + current_index = next( + index + for index, (preset_id, _meta) in enumerate(ordered_presets) + if preset_id == preset_dir.name + ) + except StopIteration: + return [] + lower_presets = [ + resolver.presets_dir / preset_id + for preset_id, _meta in ordered_presets[current_index + 1 :] + ] + lower_extensions = [ + resolver.extensions_dir / ext_id + for _priority, ext_id, _meta in resolver._get_all_extensions_by_priority() + ] + candidates = set() + for base in [*lower_presets, *lower_extensions]: + if base.parent == resolver.presets_dir: + manifest = resolver._get_manifest(base) + declarations = manifest.templates if manifest is not None else [] + else: + from ..extensions import ExtensionManifest + + manifest_path = base / "extension.yml" + try: + manifest = ( + ExtensionManifest(manifest_path) + if manifest_path.is_file() + else None + ) + except Exception: + manifest = None + declarations = [] + if manifest is not None: + provides = manifest.data.get("provides", {}) + key = {"template": "templates", "script": "scripts"}.get(resource_type) + declarations = provides.get(key, []) if key else [] + candidates.update( + item["name"] + for item in declarations + if isinstance(item, dict) + and item.get("type", resource_type) == resource_type + and isinstance(item.get("name"), str) + and not is_regex_selector(item["name"]) + ) + subdir = "templates" if resource_type == "template" else "scripts" + suffix = ".sh" if resource_type == "script" else ".md" + root = base / subdir + for path in root.glob("**/*") if root.is_dir() else []: + if path.is_file() and path.name.endswith(suffix): + candidates.add( + os.path.relpath(path, root)[: -len(suffix)].replace(os.sep, "-") + ) + suffix = ".sh" if resource_type == "script" else ".md" + core_roots = [ + resolver.templates_dir / ("scripts" if resource_type == "script" else ""), + ] + for root in core_roots: + for path in root.glob("**/*") if root.is_dir() else []: + if path.is_file() and path.name.endswith(suffix): + candidates.add( + os.path.relpath(path, root)[: -len(suffix)].replace(os.sep, "-") + ) + matches = [] + for name in candidates: + if ( + not isinstance(name, str) + or is_regex_selector(name) + or not selector_matches(selector, name) + ): + continue + exists = ( + any( + resolver._has_concrete_resource(base, name, resource_type) + for base in lower_presets + ) + or any( + resolver._has_concrete_resource( + base, name, resource_type, is_extension=True + ) + for base in lower_extensions + ) + or resolver._core_has_concrete_resource(name, resource_type) + ) + if exists: + matches.append(name) + return sorted(matches) + + @preset_app.command("info") def preset_info( preset_id: str = typer.Argument(..., help="Preset ID to get info about"), @@ -37,12 +132,44 @@ def preset_info( if isinstance(local_tags, list) and local_tags: tags_str = _escape_markup(", ".join(str(t) for t in local_tags)) console.print(f" Tags: {tags_str}") + from ._selectors import is_regex_selector + from ._resolver import PresetResolver + + resolver = PresetResolver(project_root) + preset_dir = manager.presets_dir / local_pack.id console.print(f" Templates: {len(local_pack.templates)}") for tmpl in local_pack.templates: tmpl_name = _escape_markup(str(tmpl["name"])) tmpl_type = _escape_markup(str(tmpl["type"])) tmpl_desc = _escape_markup(str(tmpl.get("description", ""))) console.print(f" - {tmpl_name} ({tmpl_type}): {tmpl_desc}") + if ( + tmpl.get("type") == "command" + and isinstance(tmpl.get("name"), str) + and is_regex_selector(tmpl["name"]) + ): + matches = manager._expand_command_selectors( + resolver, preset_dir, [tmpl] + ) + elif ( + tmpl.get("type") in {"template", "script"} + and isinstance(tmpl.get("name"), str) + and is_regex_selector(tmpl["name"]) + ): + matches = [ + {"name": name} + for name in _diagnostic_selector_matches( + resolver, preset_dir, tmpl["name"], tmpl["type"] + ) + ] + else: + matches = [] + if is_regex_selector(str(tmpl.get("name", ""))): + if matches: + for match in matches: + console.print(f" - {_escape_markup(str(match['name']))}") + else: + console.print(" [dim]No current matches[/dim]") repo = local_pack.data.get("preset", {}).get("repository") if repo: console.print(f" Repository: {_escape_markup(str(repo))}") diff --git a/src/specify_cli/presets/command_set_priority.py b/src/specify_cli/presets/command_set_priority.py index 8337c251fa..0590e86a6a 100644 --- a/src/specify_cli/presets/command_set_priority.py +++ b/src/specify_cli/presets/command_set_priority.py @@ -56,11 +56,35 @@ def preset_set_priority( old_priority = normalize_priority(raw_priority) - # Update priority + from ._resolver import PresetResolver + + resolver = PresetResolver(project_root) + affected_commands = manager._collect_selector_command_names(resolver) manager.registry.update(preset_id, {"priority": priority}) - manager.reconcile_constitution( - f"Failed to reconcile constitution after changing priority for preset {preset_id}" + affected_commands.update( + manager._collect_selector_command_names(PresetResolver(project_root)) ) + names = sorted(affected_commands) + try: + if names: + manager._reconcile_composed_commands(names) + manager._reconcile_skills(names) + manager.reconcile_constitution( + f"Failed to reconcile constitution after changing priority for preset {preset_id}" + ) + except Exception: + # Restore both registry priority and artifacts against the previous winner. + manager.registry.update(preset_id, {"priority": old_priority}) + try: + if names: + manager._reconcile_composed_commands(names) + manager._reconcile_skills(names) + manager.reconcile_constitution( + f"Failed to restore constitution after reverting priority for preset {preset_id}" + ) + except Exception: + pass + raise console.print( f"[green]✓[/green] Preset '{preset_id}' priority changed: {old_priority} → {priority}" diff --git a/tests/specify_cli/extensions/test_command_add.py b/tests/specify_cli/extensions/test_command_add.py index cddcd6bf21..b738fd39b5 100644 --- a/tests/specify_cli/extensions/test_command_add.py +++ b/tests/specify_cli/extensions/test_command_add.py @@ -38,6 +38,55 @@ class TestExtensionAddCLI: """CLI tests for ``specify extension add``.""" + @pytest.mark.parametrize("failure_point", ["hooks", "registry"]) + @pytest.mark.parametrize("force", [False, True]) + def test_install_failure_rolls_back_files_registry_and_hooks( + self, extension_dir, project_dir, monkeypatch, failure_point, force + ): + manager = ExtensionManager(project_dir) + if force: + manager.install_from_directory(extension_dir, "0.1.0", register_commands=False) + installed = manager.extensions_dir / "test-ext" + (installed / "user-data.txt").write_text("keep me", encoding="utf-8") + installed = manager.extensions_dir / "test-ext" + previous_tree = ( + {p.relative_to(installed): p.read_bytes() for p in installed.rglob("*") if p.is_file()} + if force else None + ) + previous_registry = manager.registry.registry_path.read_bytes() if manager.registry.registry_path.exists() else None + hooks_path = project_dir / ".specify" / "extensions.yml" + previous_hooks = hooks_path.read_bytes() if hooks_path.exists() else None + + if failure_point == "hooks": + original = __import__("specify_cli.extensions", fromlist=["HookExecutor"]).HookExecutor.register_hooks + + def fail_after_hooks(executor, manifest): + original(executor, manifest) + raise OSError("injected hook registration failure") + + monkeypatch.setattr( + "specify_cli.extensions.HookExecutor.register_hooks", fail_after_hooks + ) + else: + monkeypatch.setattr( + manager.registry, "add", + lambda *args, **kwargs: (_ for _ in ()).throw(OSError("injected registry failure")), + ) + + with pytest.raises(OSError, match="injected"): + manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False, force=force + ) + + assert manager.registry.registry_path.read_bytes() == previous_registry if previous_registry else not manager.registry.registry_path.exists() + assert hooks_path.read_bytes() == previous_hooks if previous_hooks else not hooks_path.exists() + if force: + assert {p.relative_to(installed): p.read_bytes() for p in installed.rglob("*") if p.is_file()} == previous_tree + assert manager.registry.is_installed("test-ext") + else: + assert not manager.registry.is_installed("test-ext") + assert not installed.exists() + def test_add_dev_links_copilot_agent_when_supported( self, extension_dir, project_dir, temp_dir ): diff --git a/tests/specify_cli/integrations/test_command_switch.py b/tests/specify_cli/integrations/test_command_switch.py index b1824102aa..7922ffc909 100644 --- a/tests/specify_cli/integrations/test_command_switch.py +++ b/tests/specify_cli/integrations/test_command_switch.py @@ -460,7 +460,13 @@ def test_switch_does_not_register_disabled_extensions(self, tmp_path): assert result.exit_code == 0, result.output opencode_git_feature = project / ".opencode" / "commands" / "speckit.git.feature.md" - assert opencode_git_feature.exists(), "Disabled extension command remains until integration switch" + assert not opencode_git_feature.exists(), "Disabling an extension removes its active artifacts" + + registry_path = project / ".specify" / "extensions" / ".registry" + registry = json.loads(registry_path.read_text(encoding="utf-8")) + git_meta = registry["extensions"]["git"] + assert git_meta["enabled"] is False + assert "opencode" not in git_meta["registered_commands"] result = _run_in_project(project, [ "integration", "switch", "claude", diff --git a/tests/specify_cli/integrations/test_command_upgrade.py b/tests/specify_cli/integrations/test_command_upgrade.py index 42d5ea0a6c..5568cfda1f 100644 --- a/tests/specify_cli/integrations/test_command_upgrade.py +++ b/tests/specify_cli/integrations/test_command_upgrade.py @@ -445,7 +445,9 @@ def test_upgrade_preserves_disabled_kilocode_legacy_extension_and_user_file( assert result.exit_code == 0, f"extension disable failed: {result.output}" disabled_extension_files = sorted(legacy.glob("speckit.git.*.md")) - assert disabled_extension_files, "disabled extension artifact should remain pre-upgrade" + assert not disabled_extension_files, ( + "disabling an extension removes its active artifacts before upgrade" + ) user_file = legacy / "speckit.user-owned.md" user_file.write_text("# user-owned legacy command", encoding="utf-8") @@ -459,11 +461,9 @@ def test_upgrade_preserves_disabled_kilocode_legacy_extension_and_user_file( assert canonical.is_dir(), ".kilo/commands/ should exist after upgrade" assert user_file.read_text(encoding="utf-8") == "# user-owned legacy command" - for disabled_file in disabled_extension_files: - assert disabled_file.exists(), ( - "disabled extension artifacts should be preserved during " - "legacy command-root reconciliation" - ) + assert not sorted(legacy.glob("speckit.git.*.md")), ( + "disabled extension artifacts should already be absent before migration" + ) assert not sorted(canonical.glob("speckit.git.*.md")), ( "disabled extensions must not be re-registered in the canonical dir" ) @@ -846,7 +846,7 @@ def test_upgrade_active_layout_change_rejected_with_disabled_preset( assert result.exit_code == 0, f"preset disable failed: {result.output}" cmd_file = commands / "speckit.plan.md" - assert "Overridden plan content" in cmd_file.read_text(encoding="utf-8") + assert "Overridden plan content" not in cmd_file.read_text(encoding="utf-8") result = _run_in_project(project, [ "integration", "upgrade", "bob", @@ -858,8 +858,8 @@ def test_upgrade_active_layout_change_rejected_with_disabled_preset( ) assert "cmd-preset" in result.output assert not skills.exists(), "no skills layout must be scaffolded on rejection" - assert "Overridden plan content" in cmd_file.read_text(encoding="utf-8"), ( - "the disabled preset's command file must be left untouched" + assert "Overridden plan content" not in cmd_file.read_text(encoding="utf-8"), ( + "disable reconciliation should remove the disabled override" ) # Enabled presets are also rejected: rescaffolding can still fail. diff --git a/tests/specify_cli/integrations/test_command_upgrade_layout.py b/tests/specify_cli/integrations/test_command_upgrade_layout.py index 671be65ac3..32e9833968 100644 --- a/tests/specify_cli/integrations/test_command_upgrade_layout.py +++ b/tests/specify_cli/integrations/test_command_upgrade_layout.py @@ -116,5 +116,10 @@ def test_installed_presets_affecting_agent_absent_vs_unreadable(self, tmp_path): ] assert _installed_command_presets_affecting_agent(project, "bob") == [ "p1", - "p7", ] + assert _installed_presets_affecting_agent( + project, "bob", include_disabled=False + ) == ["p1", "p3", "p4"] + assert _installed_presets_affecting_agent( + project, "bob", include_disabled=True + ) == ["p1", "p3", "p4", "p7"] diff --git a/tests/specify_cli/presets/test_manager.py b/tests/specify_cli/presets/test_manager.py index 91e0805593..e0d0745d42 100644 --- a/tests/specify_cli/presets/test_manager.py +++ b/tests/specify_cli/presets/test_manager.py @@ -1,6 +1,7 @@ """Tests for preset installation and removal in specify_cli.presets._manager.""" import json +import shutil import tarfile import zipfile from pathlib import Path @@ -118,6 +119,126 @@ def test_install_from_directory(self, project_dir, pack_dir): assert (installed_dir / "preset.yml").exists() assert (installed_dir / "templates" / "spec-template.md").exists() + def test_force_install_copy_failure_preserves_existing_install( + self, project_dir, pack_dir, monkeypatch + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + installed_dir = manager.presets_dir / "test-pack" + before_files = {p.relative_to(installed_dir): p.read_bytes() for p in installed_dir.rglob("*") if p.is_file()} + before_metadata = manager.registry.get("test-pack") + before_artifacts = { + p.relative_to(project_dir): p.read_bytes() + for p in project_dir.rglob("*") + if p.is_file() and ".specify/presets" not in str(p.relative_to(project_dir)) + } + real_copytree = shutil.copytree + + def fail_copytree(src, dst, *args, **kwargs): + if Path(src) == pack_dir: + raise OSError("simulated staged copy failure") + return real_copytree(src, dst, *args, **kwargs) + + monkeypatch.setattr("specify_cli.presets._manager.shutil.copytree", fail_copytree) + with pytest.raises(OSError, match="staged copy failure"): + manager.install_from_directory(pack_dir, "0.1.5", force=True) + + assert {p.relative_to(installed_dir): p.read_bytes() for p in installed_dir.rglob("*") if p.is_file()} == before_files + assert manager.registry.get("test-pack") == before_metadata + assert { + p.relative_to(project_dir): p.read_bytes() + for p in project_dir.rglob("*") + if p.is_file() and ".specify/presets" not in str(p.relative_to(project_dir)) + } == before_artifacts + + def test_force_install_registry_add_failure_restores_install_and_artifacts( + self, project_dir, pack_dir, monkeypatch + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + installed_dir = manager.presets_dir / "test-pack" + before_files = {p.relative_to(installed_dir): p.read_bytes() for p in installed_dir.rglob("*") if p.is_file()} + before_metadata = manager.registry.get("test-pack") + before_artifacts = { + p.relative_to(project_dir): p.read_bytes() + for p in project_dir.rglob("*") + if p.is_file() and ".specify/presets" not in str(p.relative_to(project_dir)) + } + + def fail_add(*args, **kwargs): + raise OSError("simulated registry pre-commit failure") + + monkeypatch.setattr(manager.registry, "add", fail_add) + with pytest.raises(OSError, match="registry pre-commit failure"): + manager.install_from_directory(pack_dir, "0.1.5", force=True) + + assert {p.relative_to(installed_dir): p.read_bytes() for p in installed_dir.rglob("*") if p.is_file()} == before_files + assert manager.registry.get("test-pack") == before_metadata + assert { + p.relative_to(project_dir): p.read_bytes() + for p in project_dir.rglob("*") + if p.is_file() and ".specify/presets" not in str(p.relative_to(project_dir)) + } == before_artifacts + + def test_force_install_selector_failure_restores_install_and_artifacts( + self, project_dir, pack_dir, monkeypatch + ): + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + installed_dir = manager.presets_dir / "test-pack" + before_install = { + p.relative_to(installed_dir): p.read_bytes() + for p in installed_dir.rglob("*") if p.is_file() + } + registry_file = manager.registry.registry_path + before_registry_file = registry_file.read_bytes() + before_metadata = manager.registry.get("test-pack") + before_artifacts = { + p.relative_to(project_dir): p.read_bytes() + for p in project_dir.rglob("*") + if p.is_file() and ".specify/presets" not in str(p.relative_to(project_dir)) + } + + def fail_selector_expansion(*args, **kwargs): + raise OSError("simulated selector expansion failure") + + monkeypatch.setattr(manager, "_expand_command_selectors", fail_selector_expansion) + with pytest.raises(OSError, match="selector expansion failure"): + manager.install_from_directory(pack_dir, "0.1.5", force=True) + + assert { + p.relative_to(installed_dir): p.read_bytes() + for p in installed_dir.rglob("*") if p.is_file() + } == before_install + assert manager.registry.get("test-pack") == before_metadata + assert registry_file.read_bytes() == before_registry_file + assert { + p.relative_to(project_dir): p.read_bytes() + for p in project_dir.rglob("*") + if p.is_file() and ".specify/presets" not in str(p.relative_to(project_dir)) + } == before_artifacts + assert not list(manager.presets_dir.glob(".test-pack.backup-*")) + + def test_initial_install_copy_failure_leaves_no_partial_install( + self, project_dir, pack_dir, monkeypatch + ): + manager = PresetManager(project_dir) + + def partial_failure(src, dst, *args, **kwargs): + Path(dst).mkdir(parents=True) + (Path(dst) / "partial").write_text("partial", encoding="utf-8") + raise OSError("simulated initial copy failure") + + monkeypatch.setattr( + "specify_cli.presets._manager.shutil.copytree", partial_failure + ) + with pytest.raises(OSError, match="initial copy failure"): + manager.install_from_directory(pack_dir, "0.1.5") + + assert not manager.registry.is_installed("test-pack") + assert not (manager.presets_dir / "test-pack").exists() + assert not list(manager.presets_dir.glob(".test-pack.stage-*")) + def test_install_already_installed(self, project_dir, pack_dir): """Test installing an already-installed pack raises error.""" manager = PresetManager(project_dir) diff --git a/tests/specify_cli/presets/test_regex_selector_diagnostics.py b/tests/specify_cli/presets/test_regex_selector_diagnostics.py new file mode 100644 index 0000000000..137f65871d --- /dev/null +++ b/tests/specify_cli/presets/test_regex_selector_diagnostics.py @@ -0,0 +1,184 @@ +from __future__ import annotations + +from pathlib import Path + +import yaml +from typer.testing import CliRunner + +from specify_cli import app +from specify_cli.presets import PresetManager, PresetRegistry +from tests.conftest import strip_ansi + + +def _write_selector_preset( + project_dir: Path, preset_id: str, templates: list[dict] +) -> Path: + preset_dir = project_dir / ".specify" / "presets" / preset_id + preset_dir.mkdir(parents=True) + for template in templates: + payload = preset_dir / template["file"] + payload.parent.mkdir(parents=True, exist_ok=True) + payload.write_text("# selector fixture\n", encoding="utf-8") + manifest = { + "schema_version": "1.0", + "preset": { + "id": preset_id, + "name": preset_id, + "version": "1.0.0", + "description": "diagnostic fixture", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"templates": templates}, + } + (preset_dir / "preset.yml").write_text(yaml.safe_dump(manifest), encoding="utf-8") + PresetRegistry(project_dir / ".specify" / "presets").add( + preset_id, {"enabled": True, "priority": 1, "version": "1.0.0"} + ) + return preset_dir + + +def _info(project_dir: Path, preset_id: str) -> str: + from unittest.mock import patch + + with patch.object(Path, "cwd", return_value=project_dir): + result = CliRunner().invoke(app, ["preset", "info", preset_id]) + assert result.exit_code == 0, result.output + return strip_ansi(result.output) + + +def test_preset_info_shows_regex_template_and_script_matches(project_dir): + # Core files are the concrete lower-layer resources for both declarations. + (project_dir / ".specify" / "templates" / "plan-template.md").write_text( + "# Core plan\n", encoding="utf-8" + ) + scripts = project_dir / ".specify" / "templates" / "scripts" + scripts.mkdir(parents=True) + (scripts / "check-prerequisites.sh").write_text("#!/bin/sh\n", encoding="utf-8") + preset_dir = _write_selector_preset( + project_dir, + "diagnostic-selectors", + [ + { + "type": "template", + "name": r"regex:^plan-.*$", + "file": "templates/overlay.md", + "strategy": "append", + }, + { + "type": "script", + "name": r"regex:^check-.*$", + "file": "scripts/overlay.sh", + "strategy": "wrap", + }, + ], + ) + + output = _info(project_dir, preset_dir.name) + + assert r"regex:^plan-.*$" in output + assert "plan-template" in output + assert r"regex:^check-.*$" in output + assert "check-prerequisites" in output + assert "No current matches" not in output + + +def test_preset_info_reports_unmatched_regex_selector(project_dir): + preset_dir = _write_selector_preset( + project_dir, + "empty-diagnostic-selector", + [ + { + "type": "template", + "name": r"regex:^nothing-matches$", + "file": "templates/overlay.md", + "strategy": "append", + } + ], + ) + + output = _info(project_dir, preset_dir.name) + + assert r"regex:^nothing-matches$" in output + assert "No current matches" in output + + +def test_preset_info_expands_regex_command_to_concrete_names(project_dir): + preset_dir = _write_selector_preset( + project_dir, + "command-diagnostic-selector", + [ + { + "type": "command", + "name": r"regex:^speckit\.(plan|tasks)$", + "file": "commands/override.md", + "strategy": "replace", + } + ], + ) + + output = _info(project_dir, preset_dir.name) + + assert r"regex:^speckit\.(plan|tasks)$" in output + assert "speckit.plan" in output + assert "speckit.tasks" in output + assert "No current matches" not in output + + +def test_install_regex_commands_registers_concrete_ai_skills(project_dir): + from specify_cli import save_init_options + + (project_dir / ".claude" / "skills").mkdir(parents=True) + save_init_options( + project_dir, + {"ai": "claude", "ai_skills": True, "script": "sh"}, + ) + source_dir = project_dir / "source" + source_dir.mkdir() + source = source_dir / "skill-selector" + source.mkdir() + (source / "commands").mkdir() + (source / "commands" / "override.md").write_text( + "---\ndescription: Preset override\n---\n\nConcrete selector skill.\n", + encoding="utf-8", + ) + manifest = { + "schema_version": "1.0", + "preset": { + "id": "skill-selector", + "name": "Skill selector", + "version": "1.0.0", + "description": "AI skills selector fixture", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "command", + "name": r"regex:^speckit\.(plan|tasks)$", + "file": "commands/override.md", + "strategy": "replace", + } + ] + }, + } + (source / "preset.yml").write_text(yaml.safe_dump(manifest), encoding="utf-8") + + manager = PresetManager(project_dir) + installed = manager.install_from_directory(source, "0.1.0") + + metadata = manager.registry.get(installed.id) + assert metadata is not None + assert metadata["registered_commands"] + skill_names = { + name for names in metadata["registered_skills"].values() for name in names + } + assert skill_names == {"speckit-plan", "speckit-tasks"} + for skill_name in skill_names: + skill_file = project_dir / ".claude" / "skills" / skill_name / "SKILL.md" + assert skill_file.is_file() + assert "Concrete selector skill." in skill_file.read_text(encoding="utf-8") + assert all( + not name.startswith("regex:") + for names in metadata["registered_commands"].values() + for name in names + ) diff --git a/tests/specify_cli/presets/test_regex_selector_lifecycle.py b/tests/specify_cli/presets/test_regex_selector_lifecycle.py new file mode 100644 index 0000000000..f99b8fd125 --- /dev/null +++ b/tests/specify_cli/presets/test_regex_selector_lifecycle.py @@ -0,0 +1,357 @@ +from __future__ import annotations + +import json +from pathlib import Path + +import yaml + +from specify_cli.extensions import _commands +from specify_cli.presets import PresetManager +from specify_cli.presets.command_disable import preset_disable + + +def _write_preset( + root: Path, preset_id: str, declarations: list[dict], files: dict[str, str] +) -> Path: + preset = root / preset_id + preset.mkdir(parents=True) + for relative, content in files.items(): + target = preset / relative + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content, encoding="utf-8") + (preset / "preset.yml").write_text( + yaml.safe_dump( + { + "schema_version": "1.0", + "preset": { + "id": preset_id, + "name": preset_id, + "version": "1.0.0", + "description": "test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {"templates": declarations}, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + return preset + + +def _write_core(project: Path, name: str, text: str = "Core body") -> None: + target = ( + project + / ".specify" + / "templates" + / "commands" + / f"{name.rsplit('.', 1)[-1]}.md" + ) + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(f"---\ndescription: core\n---\n{text}\n", encoding="utf-8") + + +def _active_claude(project: Path) -> None: + options = project / ".specify" / "init-options.json" + options.parent.mkdir(parents=True, exist_ok=True) + options.write_text( + json.dumps({"ai": "claude", "ai_skills": True}), encoding="utf-8" + ) + (project / ".claude" / "skills").mkdir(parents=True, exist_ok=True) + + +def test_extension_change_refresh_targets_only_active_integration( + monkeypatch, tmp_path +): + calls = [] + + class FakePresetManager: + def __init__(self, project_root): + assert project_root == tmp_path + + def register_enabled_presets_for_agent(self, agent): + calls.append(agent) + + monkeypatch.setattr( + "specify_cli._init_options.load_init_options", + lambda project_root: {"ai": "active-agent"}, + ) + monkeypatch.setattr("specify_cli.presets.PresetManager", FakePresetManager) + + _commands._refresh_presets_and_warn(tmp_path) + + assert calls == ["active-agent"] + + +def test_extension_change_refresh_skips_when_no_integration_selected( + monkeypatch, tmp_path +): + class UnexpectedPresetManager: + def __init__(self, project_root): + raise AssertionError("should not construct without an active agent") + + monkeypatch.setattr( + "specify_cli._init_options.load_init_options", lambda project_root: {} + ) + monkeypatch.setattr("specify_cli.presets.PresetManager", UnexpectedPresetManager) + + _commands._refresh_presets_and_warn(tmp_path) + + +def test_disabling_preset_reconciles_registered_artifacts(monkeypatch, tmp_path): + state = {"enabled": True} + calls = [] + + class FakeRegistry: + def list_by_priority(self, include_disabled=False): + return [("demo", {"enabled": state["enabled"]})] + + def is_installed(self, preset_id): + return preset_id == "demo" + + def get(self, preset_id): + return {"enabled": state["enabled"], "registered_commands": {}} + + def update(self, preset_id, updates): + state.update(updates) + calls.append(("registry", updates)) + + class FakePresetManager: + def __init__(self, project_root): + self.registry = FakeRegistry() + + def _collect_selector_command_names(self, resolver): + return {"speckit.plan"} + + def _expand_command_selectors(self, resolver, preset_dir, declarations): + return declarations + + @property + def presets_dir(self): + return tmp_path / ".specify" / "presets" + + def _skill_names_for_command(self, command_name): + return [command_name.replace(".", "-")] + + def _reconcile_composed_commands(self, names): + calls.append(("commands", names)) + + def _reconcile_skills(self, names): + calls.append(("skills", names)) + + def reconcile_constitution(self, message): + calls.append(("constitution", message)) + + monkeypatch.setattr("specify_cli._require_specify_project", lambda: tmp_path) + monkeypatch.setattr("specify_cli.presets.PresetManager", FakePresetManager) + + preset_disable("demo") + + assert state["enabled"] is False + assert calls[0][0] == "commands" + assert ("commands", ["speckit.plan"]) in calls + assert ("skills", ["speckit.plan"]) in calls + assert calls[-1][0] == "constitution" + + +def _run_preset_command(project: Path, *args: str) -> None: + from unittest.mock import patch + from typer.testing import CliRunner + from specify_cli import app + + with patch("specify_cli._require_specify_project", return_value=project): + result = CliRunner().invoke(app, ["preset", *args]) + assert result.exit_code == 0, result.output + + +def test_disabling_sole_layer_custom_preset_cleans_tracked_artifacts(tmp_path): + project = tmp_path / "project" + project.mkdir() + _active_claude(project) + source = _write_preset( + tmp_path, + "sole-owner", + [ + { + "type": "command", + "name": "speckit.custom.only", + "file": "commands/only.md", + "description": "custom only", + } + ], + {"commands/only.md": "---\ndescription: custom only\n---\nOnly body\n"}, + ) + manager = PresetManager(project) + manager.install_from_directory(source, "0.1.5") + + skill = project / ".claude" / "skills" / "speckit-custom-only" / "SKILL.md" + assert skill.is_file() + metadata = manager.registry.get("sole-owner") + assert metadata["registered_commands"]["claude"] == ["speckit.custom.only"] + assert metadata["registered_skills"]["claude"] == ["speckit-custom-only"] + + _run_preset_command(project, "disable", "sole-owner") + + assert not skill.exists() + metadata = PresetManager(project).registry.get("sole-owner") + assert metadata["enabled"] is False + assert metadata["registered_commands"] == {} + assert metadata["registered_skills"] == {} + + +def test_failed_zero_layer_cleanup_preserves_registry_provenance(monkeypatch, tmp_path): + project = tmp_path / "project" + project.mkdir() + _active_claude(project) + source = _write_preset( + tmp_path, + "cleanup-failure-owner", + [ + { + "type": "command", + "name": "speckit.custom.failure", + "file": "commands/failure.md", + "description": "cleanup failure", + } + ], + { + "commands/failure.md": "---\ndescription: cleanup failure\n---\nFailure body\n" + }, + ) + manager = PresetManager(project) + manager.install_from_directory(source, "0.1.5") + skill = project / ".claude" / "skills" / "speckit-custom-failure" / "SKILL.md" + assert skill.is_file() + assert manager.registry.get("cleanup-failure-owner")["registered_skills"][ + "claude" + ] == ["speckit-custom-failure"] + + original = PresetManager._reconcile_composed_commands + calls = 0 + + def fail_during_disabled_reconciliation(self, names, *args, **kwargs): + nonlocal calls + calls += 1 + if calls == 2: + raise OSError("cleanup blocked") + return original(self, names, *args, **kwargs) + + monkeypatch.setattr( + PresetManager, + "_reconcile_composed_commands", + fail_during_disabled_reconciliation, + ) + _run_preset_command(project, "disable", "cleanup-failure-owner") + + metadata = PresetManager(project).registry.get("cleanup-failure-owner") + assert metadata["enabled"] is False + assert metadata["registered_commands"]["claude"] == ["speckit.custom.failure"] + assert metadata["registered_skills"]["claude"] == ["speckit-custom-failure"] + assert skill.is_file() + + +def test_regex_selector_disable_and_reenable_materializes_real_artifacts(tmp_path): + project = tmp_path / "project" + project.mkdir() + _active_claude(project) + _write_core(project, "speckit.plan") + source = _write_preset( + tmp_path, + "regex-owner", + [ + { + "type": "command", + "name": r"regex:^speckit\.plan$", + "file": "commands/plan.md", + "description": "selector plan", + } + ], + {"commands/plan.md": "---\ndescription: selector plan\n---\nSelector body\n"}, + ) + # Native-skill agents restore their core layer from this local file on disable. + core_file = project / ".specify" / "templates" / "commands" / "plan.md" + core_file.parent.mkdir(parents=True, exist_ok=True) + core_file.write_text( + "---\ndescription: core plan\n---\nCore body\n", encoding="utf-8" + ) + manager = PresetManager(project) + manager.install_from_directory(source, "0.1.5") + skill = project / ".claude" / "skills" / "speckit-plan" / "SKILL.md" + assert skill.exists() and "Selector body" in skill.read_text(encoding="utf-8") + metadata = manager.registry.get("regex-owner") + assert "speckit.plan" in metadata["registered_commands"]["claude"] + assert "speckit-plan" in metadata["registered_skills"]["claude"] + assert all( + "regex:" not in name for name in metadata["registered_commands"]["claude"] + ) + assert all("regex:" not in name for name in metadata["registered_skills"]["claude"]) + + _run_preset_command(project, "disable", "regex-owner") + assert "Core body" in skill.read_text(encoding="utf-8") + assert "Selector body" not in skill.read_text(encoding="utf-8") + metadata = PresetManager(project).registry.get("regex-owner") + assert not metadata.get("registered_commands", {}).get("claude") + assert not metadata.get("registered_skills", {}).get("claude") + + _run_preset_command(project, "enable", "regex-owner") + assert skill.exists() + metadata = PresetManager(project).registry.get("regex-owner") + assert "speckit.plan" in metadata["registered_commands"]["claude"] + assert "speckit-plan" in metadata["registered_skills"]["claude"] + + +def test_regex_composition_is_concretely_tracked_and_removed(tmp_path): + project = tmp_path / "project" + project.mkdir() + _active_claude(project) + _write_core(project, "speckit.plan") + exact = _write_preset( + tmp_path, + "exact-owner", + [ + { + "type": "command", + "name": "speckit.plan", + "file": "commands/plan.md", + "description": "exact", + } + ], + {"commands/plan.md": "---\ndescription: exact\n---\nExact body\n"}, + ) + regex = _write_preset( + tmp_path, + "regex-compose", + [ + { + "type": "command", + "name": r"regex:^speckit\.plan$", + "file": "commands/plan.md", + "description": "regex composed", + "strategy": "append", + "aliases": ["plan-alias"], + } + ], + { + "commands/plan.md": "---\ndescription: regex composed\nstrategy: append\n---\nRegex addition\n" + }, + ) + manager = PresetManager(project) + manager.install_from_directory(exact, "0.1.5", priority=20) + manager.install_from_directory(regex, "0.1.5", priority=10) + skill = project / ".claude" / "skills" / "speckit-plan" / "SKILL.md" + assert skill.exists() + assert "Exact body" in skill.read_text(encoding="utf-8") + assert "Regex addition" in skill.read_text(encoding="utf-8") + metadata = manager.registry.get("regex-compose") + assert "speckit.plan" in metadata["registered_commands"]["claude"] + assert "speckit-plan" in metadata["registered_skills"]["claude"] + assert all( + "regex:" not in name for name in metadata["registered_commands"]["claude"] + ) + _run_preset_command(project, "set-priority", "regex-compose", "30") + assert "Regex addition" not in skill.read_text(encoding="utf-8") + _run_preset_command(project, "set-priority", "regex-compose", "10") + assert "Regex addition" in skill.read_text(encoding="utf-8") + assert manager.remove("regex-compose") is True + assert skill.exists() + assert "Regex addition" not in skill.read_text(encoding="utf-8") diff --git a/tests/specify_cli/presets/test_regex_selectors.py b/tests/specify_cli/presets/test_regex_selectors.py new file mode 100644 index 0000000000..7a7cea03d6 --- /dev/null +++ b/tests/specify_cli/presets/test_regex_selectors.py @@ -0,0 +1,692 @@ +from __future__ import annotations + +import json + +import pytest +import yaml + +from specify_cli.extensions import CORE_COMMAND_NAMES, ExtensionRegistry +from specify_cli.presets import ( + PresetManager, + PresetManifest, + PresetRegistry, + PresetResolver, + PresetValidationError, +) +from specify_cli.presets._selectors import selector_matches + + +def _manifest( + name: str, resource_type: str = "template", strategy: str = "replace" +) -> dict: + return { + "schema_version": "1.0", + "preset": { + "id": "preset", + "name": "Preset", + "version": "1.0.0", + "description": "test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": resource_type, + "name": name, + "file": "payload.md" if resource_type != "script" else "payload.sh", + "strategy": strategy, + } + ] + }, + } + + +def _write_preset( + project, + preset_id: str, + name: str, + *, + priority: int, + strategy: str = "replace", + resource_type: str = "template", + body: str = "overlay\n", +): + root = project / ".specify" / "presets" / preset_id + root.mkdir(parents=True, exist_ok=True) + data = _manifest(name, resource_type, strategy) + data["preset"]["id"] = preset_id + filename = data["provides"]["templates"][0]["file"] + (root / filename).write_text(body, encoding="utf-8") + (root / "preset.yml").write_text(yaml.safe_dump(data), encoding="utf-8") + PresetRegistry(project / ".specify" / "presets").add( + preset_id, {"enabled": True, "priority": priority, "version": "1.0.0"} + ) + return root + + +def test_command_regex_expands_core_commands_to_exact_names(project_dir): + selector_pack = _write_preset( + project_dir, + "command-selector", + r"regex:^speckit\.(plan|tasks)$", + priority=1, + resource_type="command", + ) + manager = PresetManager(project_dir) + manifest = PresetManifest(selector_pack / "preset.yml") + expanded = manager._expand_command_selectors( + PresetResolver(project_dir), selector_pack, manifest.templates + ) + names = {entry["name"] for entry in expanded} + expected = {f"speckit.{name}" for name in CORE_COMMAND_NAMES} + assert names == expected & {"speckit.plan", "speckit.tasks"} + + +def test_command_regex_expands_extension_command(project_dir): + ext = project_dir / ".specify" / "extensions" / "demo" + (ext / "commands").mkdir(parents=True) + (ext / "commands" / "speckit.demo.md").write_text( + "extension command\\n", encoding="utf-8" + ) + _write_preset( + project_dir, + "command-selector", + r"regex:^speckit\.demo$", + priority=1, + resource_type="command", + ) + ExtensionRegistry(project_dir / ".specify" / "extensions").add( + "demo", {"enabled": True, "priority": 10, "version": "1.0"} + ) + pack = project_dir / ".specify" / "presets" / "command-selector" + manager = PresetManager(project_dir) + manifest = PresetManifest(pack / "preset.yml") + expanded = manager._expand_command_selectors( + PresetResolver(project_dir), pack, manifest.templates + ) + assert [entry["name"] for entry in expanded] == ["speckit.demo"] + + +def test_command_regex_expands_lower_preset_and_excludes_own_or_higher(project_dir): + lower = project_dir / ".specify" / "presets" / "lower" + lower.mkdir(parents=True) + (lower / "commands").mkdir() + (lower / "payload.md").write_text("base\n", encoding="utf-8") + lower_data = _manifest("speckit.lower", "command") + lower_data["preset"]["id"] = "lower" + lower_data["provides"]["templates"][0]["file"] = "payload.md" + (lower / "preset.yml").write_text(yaml.safe_dump(lower_data), encoding="utf-8") + PresetRegistry(project_dir / ".specify" / "presets").add( + "lower", {"enabled": True, "priority": 10, "version": "1.0"} + ) + selector_pack = _write_preset( + project_dir, + "selector", + r"regex:^speckit\.lower$", + priority=1, + resource_type="command", + ) + manager = PresetManager(project_dir) + manifest = PresetManifest(selector_pack / "preset.yml") + expanded = manager._expand_command_selectors( + PresetResolver(project_dir), selector_pack, manifest.templates + ) + assert [entry["name"] for entry in expanded] == ["speckit.lower"] + + +def test_command_regex_and_regex_overlap_preserves_both_entries(project_dir): + selector_pack = _write_preset( + project_dir, + "double-selector", + r"regex:^speckit\.plan$", + priority=1, + resource_type="command", + ) + second = {"type": "command", "name": r"regex:^speckit\.plan$", "file": "payload.md"} + manager = PresetManager(project_dir) + manifest = PresetManifest(selector_pack / "preset.yml") + expanded = manager._expand_command_selectors( + PresetResolver(project_dir), selector_pack, [*manifest.templates, second] + ) + assert [entry["name"] for entry in expanded] == ["speckit.plan", "speckit.plan"] + + +def test_command_regex_registration_tracks_and_cleans_only_concrete_names(project_dir): + ( + project_dir / ".specify" / "templates" / "commands" / "speckit.plan.md" + ).write_text("# Plan\n", encoding="utf-8") + ( + project_dir / ".specify" / "templates" / "commands" / "speckit.tasks.md" + ).write_text("# Tasks\n", encoding="utf-8") + commands_dir = project_dir / ".agents" / "commands" + commands_dir.mkdir(parents=True) + (project_dir / ".specify" / "init-options.json").write_text( + json.dumps({"ai": "amp", "ai_skills": False, "script": "sh"}), + encoding="utf-8", + ) + source_dir = project_dir / "selector-source" + source_dir.mkdir() + source = _write_preset( + source_dir, + "command-selector", + r"regex:^speckit\.(plan|tasks)$", + priority=10, + resource_type="command", + ) + # Core discovery in this isolated project uses the packaged command inventory. + manager = PresetManager(project_dir) + source = source_dir / ".specify" / "presets" / "command-selector" + manifest = manager.install_from_directory(source, "0.1.0") + metadata = manager.registry.get(manifest.id) + registered = metadata["registered_commands"] + tracked = {name for names in registered.values() for name in names} + expected = {f"speckit.{name}" for name in CORE_COMMAND_NAMES} & { + "speckit.plan", + "speckit.tasks", + } + assert tracked == expected + assert all(not name.startswith("regex:") for name in tracked) + files = list(commands_dir.iterdir()) + assert files + assert all(path.exists() for path in files) + assert all( + path.name.startswith(("speckit.plan", "speckit.tasks")) for path in files + ) + assert all(not path.name.startswith("regex:") for path in files) + manager.remove(manifest.id) + + +def test_command_regex_zero_match_and_fullmatch(project_dir): + selector_pack = _write_preset( + project_dir, + "no-command-match", + r"regex:^missing\.command$", + priority=1, + resource_type="command", + ) + manager = PresetManager(project_dir) + manifest = PresetManifest(selector_pack / "preset.yml") + assert ( + manager._expand_command_selectors( + PresetResolver(project_dir), selector_pack, manifest.templates + ) + == [] + ) + + +def test_command_regex_and_exact_overlap_keep_only_concrete_names(project_dir): + _write_preset( + project_dir, + "overlap-selector", + r"regex:^speckit\.plan$", + priority=1, + resource_type="command", + ) + selector_pack = project_dir / ".specify" / "presets" / "overlap-selector" + exact = {"type": "command", "name": "speckit.plan", "file": "commands/exact.md"} + manager = PresetManager(project_dir) + manifest = PresetManifest(selector_pack / "preset.yml") + expanded = manager._expand_command_selectors( + PresetResolver(project_dir), selector_pack, [exact, *manifest.templates] + ) + assert "regex:" not in " ".join(entry["name"] for entry in expanded) + assert [entry["name"] for entry in expanded].count("speckit.plan") == 2 + + +def test_exact_name_manifest_still_validates(tmp_path): + path = tmp_path / "preset.yml" + path.write_text(yaml.safe_dump(_manifest("plan-template")), encoding="utf-8") + manifest = PresetManifest(path) + assert manifest.templates[0]["name"] == "plan-template" + assert selector_matches("plan-template", "plan-template") + assert not selector_matches("plan-template", "other-template") + + +def test_exact_name_manifest_keeps_legacy_trailing_newline_validation(tmp_path): + # Legacy `re.match(r'^[...]$')` accepts one trailing newline; preserve it. + path = tmp_path / "preset.yml" + path.write_text(yaml.safe_dump(_manifest("plan-template\n")), encoding="utf-8") + manifest = PresetManifest(path) + assert manifest.templates[0]["name"] == "plan-template\n" + + +def test_regex_validation_and_fullmatch(tmp_path): + path = tmp_path / "preset.yml" + selector = r"regex:^plan-.*-template$" + path.write_text(yaml.safe_dump(_manifest(selector)), encoding="utf-8") + assert PresetManifest(path).templates[0]["name"] == selector + assert selector_matches(selector, "plan-main-template") + assert not selector_matches("regex:plan", "plan-template") + + +def _write_command_declarations(pack_dir, preset_id, declarations): + pack_dir.mkdir(parents=True, exist_ok=True) + for index, declaration in enumerate(declarations): + payload = pack_dir / f"command-{index}.md" + payload.write_text(declaration["body"], encoding="utf-8") + declaration["file"] = payload.name + declaration.pop("body") + data = _manifest("unused", "command") + data["preset"]["id"] = preset_id + data["provides"]["templates"] = declarations + (pack_dir / "preset.yml").write_text(yaml.safe_dump(data), encoding="utf-8") + return pack_dir + + +def _install_test_preset(project_dir, preset_id, pack_dir, priority): + PresetRegistry(project_dir / ".specify" / "presets").add( + preset_id, {"enabled": True, "priority": priority, "version": "1.0.0"} + ) + destination = project_dir / ".specify" / "presets" / preset_id + destination.mkdir(parents=True, exist_ok=True) + for path in pack_dir.iterdir(): + (destination / path.name).write_bytes(path.read_bytes()) + return destination + + +@pytest.mark.parametrize( + ("strategy", "overlay", "expected"), + [ + ("replace", "# Preset replacement\n", "# Preset replacement"), + ("prepend", "# Preset prefix\n", "# Preset prefix\n\n\n# Core command"), + ("append", "# Preset suffix\n", "# Core command\n\n\n# Preset suffix"), + ( + "wrap", + "# Wrapper start\n{CORE_TEMPLATE}\n# Wrapper end\n", + "# Wrapper start\n# Core command\n\n# Wrapper end", + ), + ], +) +def test_command_regex_composes_over_core_for_each_strategy( + project_dir, tmp_path, strategy, overlay, expected +): + core_commands = project_dir / ".specify" / "templates" / "commands" + core_commands.mkdir(parents=True, exist_ok=True) + (core_commands / "plan.md").write_text("# Core command\n", encoding="utf-8") + pack_dir = tmp_path / "selector" + declaration = { + "type": "command", + "name": r"regex:^speckit\.plan$", + "strategy": strategy, + "body": overlay, + } + _write_command_declarations(pack_dir, "selector", [declaration]) + installed = _install_test_preset(project_dir, "selector", pack_dir, 1) + + content = PresetResolver(project_dir).resolve_content("speckit.plan", "command") + + assert content is not None + assert content.strip() == expected.strip() + layers = PresetResolver(project_dir).collect_all_layers("speckit.plan", "command") + assert [layer["source"] for layer in layers] == ["selector v1.0.0", "core"] + assert layers[0]["path"] == installed / "command-0.md" + + +def test_command_regex_expands_and_composes_over_extension_layer(project_dir, tmp_path): + extension = project_dir / ".specify" / "extensions" / "demo" + (extension / "commands").mkdir(parents=True) + (extension / "commands" / "speckit.demo.md").write_text( + "# Extension command\n", encoding="utf-8" + ) + ExtensionRegistry(project_dir / ".specify" / "extensions").add( + "demo", {"enabled": True, "priority": 10, "version": "1.0"} + ) + pack_dir = tmp_path / "selector" + _write_command_declarations( + pack_dir, + "selector", + [ + { + "type": "command", + "name": r"regex:^speckit\.demo$", + "strategy": "append", + "body": "# Preset addition\n", + } + ], + ) + _install_test_preset(project_dir, "selector", pack_dir, 1) + + content = PresetResolver(project_dir).resolve_content("speckit.demo", "command") + + assert content == "# Extension command\n\n\n# Preset addition\n" + layers = PresetResolver(project_dir).collect_all_layers("speckit.demo", "command") + assert [layer["source"] for layer in layers] == [ + "selector v1.0.0", + "extension:demo v1.0", + ] + + +def test_command_regex_expands_and_composes_over_lower_preset(project_dir, tmp_path): + lower_pack = tmp_path / "lower" + _write_command_declarations( + lower_pack, + "lower", + [ + { + "type": "command", + "name": "speckit.lower", + "strategy": "replace", + "body": "# Lower preset command\n", + } + ], + ) + _install_test_preset(project_dir, "lower", lower_pack, 10) + selector_pack = tmp_path / "selector" + _write_command_declarations( + selector_pack, + "selector", + [ + { + "type": "command", + "name": r"regex:^speckit\.lower$", + "strategy": "prepend", + "body": "# Selector prefix\n", + } + ], + ) + _install_test_preset(project_dir, "selector", selector_pack, 1) + + content = PresetResolver(project_dir).resolve_content("speckit.lower", "command") + + assert content == "# Selector prefix\n\n\n# Lower preset command\n" + layers = PresetResolver(project_dir).collect_all_layers("speckit.lower", "command") + assert [layer["source"] for layer in layers] == [ + "selector v1.0.0", + "lower v1.0.0", + ] + + +def test_command_regex_excludes_higher_preset_and_project_override( + project_dir, tmp_path +): + # The selector is lower priority than both the higher preset and the project + # override. Neither may make a command eligible for selector expansion. + higher_pack = tmp_path / "higher" + _write_command_declarations( + higher_pack, + "higher", + [ + { + "type": "command", + "name": "speckit.higher", + "strategy": "replace", + "body": "# Higher command\n", + } + ], + ) + _install_test_preset(project_dir, "higher", higher_pack, 1) + selector_pack = tmp_path / "selector" + _write_command_declarations( + selector_pack, + "selector", + [ + { + "type": "command", + "name": r"regex:^speckit\.(higher|override)$", + "strategy": "append", + "body": "# Selector addition\n", + } + ], + ) + installed = _install_test_preset(project_dir, "selector", selector_pack, 10) + override_dir = project_dir / ".specify" / "templates" / "overrides" + override_dir.mkdir(parents=True) + (override_dir / "speckit.override.md").write_text("# Project override\n") + manager = PresetManager(project_dir) + manifest = PresetManifest(installed / "preset.yml") + + expanded = manager._expand_command_selectors( + PresetResolver(project_dir), installed, manifest.templates + ) + + assert expanded == [] + assert ( + PresetResolver(project_dir).collect_all_layers("speckit.higher", "command")[0][ + "source" + ] + == "higher v1.0.0" + ) + assert ( + PresetResolver(project_dir).collect_all_layers("speckit.override", "command")[ + 0 + ]["source"] + == "project override" + ) + + +def test_three_overlapping_command_regexes_keep_declaration_order_and_compose( + project_dir, tmp_path +): + core_commands = project_dir / ".specify" / "templates" / "commands" + core_commands.mkdir(parents=True, exist_ok=True) + (core_commands / "plan.md").write_text("# Core command\n", encoding="utf-8") + pack_dir = tmp_path / "selector" + declarations = [ + { + "type": "command", + "name": r"regex:^speckit\.plan$", + "strategy": "prepend", + "body": "# Declared first\n", + }, + { + "type": "command", + "name": r"regex:^speckit\.p.*$", + "strategy": "append", + "body": "# Declared second\n", + }, + { + "type": "command", + "name": r"regex:^speckit\.pla.*n$", + "strategy": "wrap", + "body": "# Declared third start\n{CORE_TEMPLATE}\n# Declared third end\n", + }, + ] + _write_command_declarations(pack_dir, "selector", declarations) + _install_test_preset(project_dir, "selector", pack_dir, 1) + manager = PresetManager(project_dir) + manifest = PresetManifest( + project_dir / ".specify" / "presets" / "selector" / "preset.yml" + ) + + expanded = manager._expand_command_selectors( + PresetResolver(project_dir), + project_dir / ".specify" / "presets" / "selector", + manifest.templates, + ) + layers = PresetResolver(project_dir).collect_all_layers("speckit.plan", "command") + content = PresetResolver(project_dir).resolve_content("speckit.plan", "command") + + assert [entry["name"] for entry in expanded] == [ + "speckit.plan", + "speckit.plan", + "speckit.plan", + ] + assert [layer["path"].name for layer in layers] == [ + "command-0.md", + "command-1.md", + "command-2.md", + "plan.md", + ] + assert content is not None + assert content.index("Declared first") < content.index("Declared third start") + assert content.index("Declared third start") < content.index("Core command") + assert content.index("Core command") < content.index("Declared third end") + assert content.index("Declared third end") < content.index("Declared second") + + +def test_invalid_regex_fails_during_manifest_validation(tmp_path): + path = tmp_path / "preset.yml" + path.write_text(yaml.safe_dump(_manifest("regex:[unterminated")), encoding="utf-8") + with pytest.raises(PresetValidationError, match="Invalid regex selector"): + PresetManifest(path) + + +def test_template_regex_matches_core_resource_only(project_dir): + _write_preset( + project_dir, "regex-pack", "regex:.*-template$", priority=10, strategy="append" + ) + layers = PresetResolver(project_dir).collect_all_layers("plan-template", "template") + assert any(layer["source"].startswith("regex-pack") for layer in layers) + assert ( + PresetResolver(project_dir).collect_all_layers("does-not-exist", "template") + == [] + ) + + +def test_template_regex_matches_extension_resource(project_dir): + ext = project_dir / ".specify" / "extensions" / "demo" + (ext / "templates").mkdir(parents=True) + (ext / "templates" / "plan-template.md").write_text( + "extension base\n", encoding="utf-8" + ) + _write_preset( + project_dir, "regex-pack", "regex:.*-template$", priority=10, strategy="append" + ) + layers = PresetResolver(project_dir).collect_all_layers("plan-template", "template") + assert any(layer["source"].startswith("regex-pack") for layer in layers) + + +def test_template_regex_matches_lower_priority_preset(project_dir): + _write_preset( + project_dir, "high", "regex:.*-template$", priority=1, strategy="append" + ) + _write_preset(project_dir, "low", "plan-template", priority=10) + layers = PresetResolver(project_dir).collect_all_layers("plan-template", "template") + assert any(layer["source"].startswith("high") for layer in layers) + assert any(layer["source"].startswith("low") for layer in layers) + + +def test_regex_does_not_match_higher_preset_or_create_resource(project_dir): + _write_preset(project_dir, "higher", "only-higher", priority=1) + _write_preset(project_dir, "lower", "regex:^only-higher$", priority=10) + resolver = PresetResolver(project_dir) + layers = resolver.collect_all_layers("only-higher", "template") + assert any(layer["source"].startswith("higher") for layer in layers) + assert not any(layer["source"].startswith("lower") for layer in layers) + assert resolver.collect_all_layers("does-not-exist", "template") == [] + assert resolver.collect_all_layers("regex:^only-higher$", "template") == [] + + +def test_same_priority_regex_layers_follow_preset_id_order(project_dir): + # Registry tie-break is alphabetical ID; both selectors match the same core template. + _write_preset( + project_dir, + "zeta-regex", + "regex:^plan-template$", + priority=10, + strategy="append", + body="zeta\n", + ) + _write_preset( + project_dir, + "alpha-regex", + "regex:^plan-template$", + priority=10, + strategy="append", + body="alpha\n", + ) + layers = PresetResolver(project_dir).collect_all_layers("plan-template", "template") + regex_sources = [ + layer["source"].split()[0] for layer in layers if "-regex" in layer["source"] + ] + assert regex_sources == ["alpha-regex", "zeta-regex"] + + +def test_project_override_is_not_regex_lower_layer_proof(project_dir): + overrides = project_dir / ".specify" / "templates" / "overrides" + overrides.mkdir(parents=True) + (overrides / "foo-template.md").write_text("override\n", encoding="utf-8") + _write_preset( + project_dir, + "regex-pack", + "regex:^foo-template$", + priority=10, + strategy="append", + ) + resolver = PresetResolver(project_dir) + layers = resolver.collect_all_layers("foo-template", "template") + assert [layer["source"] for layer in layers] == ["project override"] + + +def test_project_override_does_not_hide_real_regex_lower_layer(project_dir): + overrides = project_dir / ".specify" / "templates" / "overrides" + overrides.mkdir(parents=True) + (overrides / "foo-template.md").write_text("override\n", encoding="utf-8") + _write_preset( + project_dir, + "regex-pack", + "regex:^foo-template$", + priority=10, + strategy="append", + ) + core = project_dir / ".specify" / "templates" / "foo-template.md" + core.parent.mkdir(parents=True, exist_ok=True) + core.write_text("core\n", encoding="utf-8") + layers = PresetResolver(project_dir).collect_all_layers("foo-template", "template") + assert [layer["source"] for layer in layers] == [ + "project override", + "regex-pack v1.0.0", + "core", + ] + + +def test_exact_and_regex_layers_keep_existing_priority_composition(project_dir): + _write_preset( + project_dir, + "higher-regex", + "regex:^plan-template$", + priority=1, + strategy="append", + body="A\n", + ) + _write_preset( + project_dir, + "higher-exact", + "plan-template", + priority=2, + strategy="prepend", + body="B\n", + ) + resolver = PresetResolver(project_dir) + layers = resolver.collect_all_layers("plan-template", "template") + assert [layer["source"].split(" ")[0] for layer in layers[:2]] == [ + "higher-regex", + "higher-exact", + ] + result = resolver.resolve_content("plan-template", "template") + assert result is not None and "A" in result and "B" in result + + +def test_script_regex_uses_same_resolver_and_strategy_restrictions(project_dir): + core_scripts = project_dir / ".specify" / "templates" / "scripts" + core_scripts.mkdir(parents=True) + (core_scripts / "check-main.sh").write_text( + "#!/bin/sh\necho core\n", encoding="utf-8" + ) + _write_preset( + project_dir, + "script-regex", + "regex:^check-.*$", + priority=10, + strategy="wrap", + resource_type="script", + body="#!/bin/sh\n{CORE_SCRIPT}\n", + ) + resolver = PresetResolver(project_dir) + assert any( + layer["source"].startswith("script-regex") + for layer in resolver.collect_all_layers("check-main", "script") + ) + invalid = _manifest("regex:^check-.*$", "script", "append") + path = project_dir / "invalid-script-preset.yml" + path.write_text(yaml.safe_dump(invalid), encoding="utf-8") + with pytest.raises(PresetValidationError, match="scripts only support"): + PresetManifest(path) + + +def test_zero_match_is_non_fatal_and_has_no_layer(project_dir): + _write_preset(project_dir, "no-match", "regex:^missing-resource$", priority=10) + assert ( + PresetResolver(project_dir).collect_all_layers("missing-resource", "template") + == [] + ) diff --git a/tests/specify_cli/presets/test_resolver.py b/tests/specify_cli/presets/test_resolver.py index d1e416f7a1..06611ca20e 100644 --- a/tests/specify_cli/presets/test_resolver.py +++ b/tests/specify_cli/presets/test_resolver.py @@ -642,6 +642,108 @@ def test_resolve_extension_command_via_manifest_skips_oserror_manifests(self, pr assert result is None, "OSError during manifest load must be silently skipped" + def test_collect_all_layers_extension_alternate_filename_respects_registry( + self, project_dir + ): + extensions_dir = project_dir / ".specify" / "extensions" + ext_dir = extensions_dir / "alternate" + command_file = ext_dir / "commands" / "alternate.collect.md" + command_file.parent.mkdir(parents=True) + command_file.write_text("# alternate command\n", encoding="utf-8") + (ext_dir / "extension.yml").write_text( + yaml.safe_dump( + { + "schema_version": "1.0", + "extension": { + "id": "alternate", + "name": "alternate", + "version": "1.0.0", + "description": "alternate command fixture", + }, + "provides": { + "commands": [ + { + "name": "speckit.alternate.collect", + "file": "commands/alternate.collect.md", + "description": "collect", + } + ] + }, + } + ), + encoding="utf-8", + ) + registry = ExtensionRegistry(extensions_dir) + registry.add("alternate", {"enabled": True, "version": "1.0.0"}) + resolver = PresetResolver(project_dir) + + enabled_layers = resolver.collect_all_layers( + "speckit.alternate.collect", "command" + ) + assert any(layer["path"] == command_file for layer in enabled_layers) + + registry.update("alternate", {"enabled": False}) + disabled_layers = resolver.collect_all_layers( + "speckit.alternate.collect", "command" + ) + assert all(layer["path"] != command_file for layer in disabled_layers) + + def test_unregistered_extension_alternate_command_fallback_remains_supported( + self, project_dir + ): + ext_dir = project_dir / ".specify" / "extensions" / "orphan" + command_file = ext_dir / "commands" / "orphan.collect.md" + command_file.parent.mkdir(parents=True) + command_file.write_text("# orphan command\n", encoding="utf-8") + + layers = PresetResolver(project_dir).collect_all_layers( + "speckit.orphan.collect", "command" + ) + + assert any(layer["path"] == command_file for layer in layers) + + def test_collect_all_layers_extension_alternate_manifest_must_own_command( + self, project_dir + ): + ext_dir = project_dir / ".specify" / "extensions" / "named" + command_file = ext_dir / "commands" / "named.other.md" + command_file.parent.mkdir(parents=True) + command_file.write_text("# unrelated file\n", encoding="utf-8") + (ext_dir / "extension.yml").write_text( + yaml.safe_dump( + { + "schema_version": "1.0", + "extension": { + "id": "named", + "name": "named", + "version": "1.0.0", + "description": "manifest ownership fixture", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.named.declared", + "file": "commands/named.other.md", + "description": "declared", + } + ] + }, + } + ), + encoding="utf-8", + ) + ExtensionRegistry(project_dir / ".specify" / "extensions").add( + "named", {"enabled": True, "version": "1.0.0"} + ) + + layers = PresetResolver(project_dir).collect_all_layers( + "speckit.named.declared", "command" + ) + + assert any(layer["path"] == command_file for layer in layers) + + class TestExtensionPriorityResolution: """Test extension priority resolution with registered and unregistered extensions."""