From b674156c799f326fc9bb947e9cfcd0ba9a653471 Mon Sep 17 00:00:00 2001 From: Yash-Chindam Date: Fri, 21 Aug 2026 08:53:15 +0530 Subject: [PATCH 1/3] feat(presets): let a preset declare a required extension A preset whose command overrides call into an extension is inert without it, but the overrides fall through to the core workflow, so nothing errors -- the feature just silently does less than the user expects. Until now the only place that dependency could be stated was the README, which fails exactly the user who did not read it. Add an optional requires.extensions to preset.yml, accepting either a bare extension id or a mapping with an optional version specifier and an optional required flag. Validation mirrors the requires.speckit_version strictness from #3980: a non-list, a member that is neither string nor mapping, a missing or malformed id, a non-string or unparseable version, and a non-boolean required each raise PresetValidationError rather than surfacing later as a bare TypeError from re.match or SpecifierSet. On `specify preset add`, warn once for each unsatisfied dependency, naming the extension and the command that installs it. The check runs at the single point where the --dev, --from, and catalog paths converge, so all three behave the same. It warns rather than fails: these presets are written to degrade safely, and three catalog entries already declare the dependency, so failing would break installs that work today. The field is optional, so every existing preset stays valid and silent. Closes #4231 Assisted-by: Claude Code (model: Claude Opus 5, autonomous) --- presets/PUBLISHING.md | 30 +++++ src/specify_cli/presets/__init__.py | 172 ++++++++++++++++++++++++ src/specify_cli/presets/_commands.py | 39 ++++++ tests/test_presets.py | 194 +++++++++++++++++++++++++++ 4 files changed, 435 insertions(+) diff --git a/presets/PUBLISHING.md b/presets/PUBLISHING.md index f71c1f45d8..439fd54300 100644 --- a/presets/PUBLISHING.md +++ b/presets/PUBLISHING.md @@ -68,6 +68,8 @@ preset: requires: speckit_version: ">=0.1.0" # Required spec-kit version + extensions: # Optional: extensions this preset needs + - "companion-extension" provides: templates: @@ -93,6 +95,34 @@ tags: # 2-5 relevant tags - ✅ Command names use dot notation (e.g. `speckit.specify`) - ✅ Tags are lowercase and descriptive +#### Declaring extension dependencies + +If your preset overrides commands that call into an extension, declare it in +`requires.extensions`. Without the extension the preset still installs and the +overrides fall through to the core workflow, so nothing errors — the feature +just silently does less than the user expects. Declaring the dependency makes +`specify preset add` say so, and name the command that fixes it. + +Use a bare id, or a mapping when you need a version floor or an optional +dependency: + +```yaml +requires: + speckit_version: ">=0.9.0" + extensions: + - "companion-extension" # required, any version + - id: "other-extension" + version: ">=1.2.0" # optional PEP 440 specifier + required: false # optional, defaults to true +``` + +Notes: + +- The field is optional. A preset that declares nothing behaves exactly as before. +- A missing or version-unsatisfied dependency produces a **warning, not a failure** — the install still succeeds. +- `required: false` documents an enhancing-but-optional extension and is never warned about. +- Declare it in `preset.yml`, not only in your catalog entry. The catalog is not consulted for `--dev` and `--from ` installs, so the manifest is the only copy present on every install path. + ### 3. Test Locally ```bash diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 54dc5d2845..302221ed8a 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -381,6 +381,14 @@ def _validate(self): f"got {type(requires['speckit_version']).__name__}" ) + # Validate the optional extension dependency list. A preset that + # overrides commands calling into an extension is inert without it, and + # until now the only place that could be said was the README -- see + # issue #4231. Absent means "no dependencies", so every existing preset + # stays valid. + if "extensions" in requires: + self._validate_requires_extensions(requires["extensions"]) + # Validate provides section provides = self.data["provides"] if "templates" not in provides: @@ -524,11 +532,116 @@ def author(self) -> str: """Get preset author.""" return self.data["preset"].get("author", "") + @staticmethod + def _validate_requires_extensions(declared: Any) -> None: + """Validate the optional ``requires.extensions`` list. + + Accepts either a bare extension id or a mapping carrying an optional + version specifier and an optional ``required`` flag: + + .. code-block:: yaml + + requires: + extensions: + - speckit-inventory + - id: other-ext + version: ">=1.2.0" + required: false + + Raises: + PresetValidationError: If the list or any entry is malformed. + """ + if not isinstance(declared, list): + raise PresetValidationError( + "Invalid requires.extensions: expected a list, " + f"got {type(declared).__name__}" + ) + + for index, entry in enumerate(declared): + label = f"requires.extensions[{index}]" + + if isinstance(entry, str): + entry = {"id": entry} + elif not isinstance(entry, dict): + raise PresetValidationError( + f"Invalid {label}: expected a string or a mapping, " + f"got {type(entry).__name__}" + ) + + if "id" not in entry: + raise PresetValidationError(f"Missing {label}.id") + extension_id = entry["id"] + if not isinstance(extension_id, str): + raise PresetValidationError( + f"Invalid {label}.id: expected a string, " + f"got {type(extension_id).__name__}" + ) + # Same id shape the extension loader enforces, so a dependency can + # never name something that could not be installed in the first place. + if not re.match(r'^[a-z0-9-]+$', extension_id): + raise PresetValidationError( + f"Invalid {label}.id '{extension_id}': " + "must be lowercase alphanumeric with hyphens only" + ) + + if "version" in entry: + constraint = entry["version"] + # Mirrors the requires.speckit_version reasoning: a non-string + # escapes InvalidSpecifier two ways -- scalars raise TypeError + # from the constructor, and a list/dict is iterable so it + # constructs and only fails later inside .contains(). + if not isinstance(constraint, str) or not constraint.strip(): + raise PresetValidationError( + f"Invalid {label}.version: expected a non-empty string, " + f"got {type(constraint).__name__}" + ) + try: + SpecifierSet(constraint) + except InvalidSpecifier: + raise PresetValidationError( + f"Invalid {label}.version '{constraint}': " + "not a valid version specifier" + ) + + if "required" in entry and not isinstance(entry["required"], bool): + raise PresetValidationError( + f"Invalid {label}.required: expected a boolean, " + f"got {type(entry['required']).__name__}" + ) + @property def requires_speckit_version(self) -> str: """Get required spec-kit version range.""" return self.data["requires"]["speckit_version"] + @property + def requires_extensions(self) -> List[Dict[str, Any]]: + """Get declared extension dependencies, normalized to mappings. + + Returns: + One entry per dependency with ``id``, ``version`` (``None`` when + unconstrained), and ``required`` (defaulting to ``True``). Empty + when the manifest declares no dependencies. + """ + declared = self.data["requires"].get("extensions") + if not isinstance(declared, list): + return [] + + normalized: List[Dict[str, Any]] = [] + for entry in declared: + if isinstance(entry, str): + entry = {"id": entry} + if not isinstance(entry, dict) or not isinstance(entry.get("id"), str): + continue + normalized.append( + { + "id": entry["id"], + "version": entry.get("version"), + "required": entry.get("required", True), + } + ) + return normalized + @property def templates(self) -> List[Dict[str, Any]]: """Get list of provided templates.""" @@ -838,6 +951,65 @@ def check_compatibility( return True + def find_unmet_extension_dependencies( + self, + manifest: PresetManifest + ) -> List[Dict[str, Any]]: + """Find declared extension dependencies that are not satisfied. + + Reports rather than raises. A preset whose overrides call into an + extension is written to degrade safely -- without the extension the + core workflow still runs -- so a missing dependency is a warning, not + an install failure. See issue #4231. + + Args: + manifest: Preset manifest to inspect + + Returns: + One entry per unsatisfied dependency, each with ``id``, the + requested ``version`` specifier (``None`` when unconstrained), the + ``installed`` version (``None`` when absent), and a ``reason`` of + either ``"missing"`` or ``"version"``. Optional dependencies + (``required: false``) are never reported. + """ + # Defense in depth, mirroring check_compatibility(): this method is + # public and also reachable with a hand-built manifest object that + # predates this field. A manifest without it declares nothing. + candidates = getattr(manifest, "requires_extensions", None) + if not isinstance(candidates, list): + return [] + + declared = [ + dep for dep in candidates + if isinstance(dep, dict) and dep.get("required", True) + ] + if not declared: + return [] + + registry = ExtensionRegistry(self.project_root / ".specify" / "extensions") + unmet: List[Dict[str, Any]] = [] + + for dep in declared: + metadata = registry.get(dep["id"]) + if metadata is None: + unmet.append({**dep, "installed": None, "reason": "missing"}) + continue + + constraint = dep["version"] + if not constraint: + continue + + installed = metadata.get("version") + if not isinstance(installed, str): + # A registry entry without a usable version cannot be compared. + # Treat it as satisfied rather than inventing a failure, since + # the extension is demonstrably installed. + continue + if not version_satisfies(installed, constraint): + unmet.append({**dep, "installed": installed, "reason": "version"}) + + return unmet + def _register_commands( self, manifest: PresetManifest, diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index 48d5c9f14f..f7b02f14df 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -40,6 +40,39 @@ preset_app.add_typer(preset_catalog_app, name="catalog") +def _warn_unmet_extension_dependencies(manager, manifest) -> None: + """Warn when a preset's declared extension dependencies are unsatisfied. + + A preset whose command overrides call into an extension is inert without + it, but the overrides still fall through to the core workflow, so nothing + breaks -- it just silently does less than the user expects. Naming the + missing extension and the command that installs it turns that silence into + something actionable. See issue #4231. + """ + unmet = manager.find_unmet_extension_dependencies(manifest) + if not unmet: + return + + console.print() + console.print("[yellow]![/yellow] This preset depends on extensions that are not satisfied:") + for dep in unmet: + extension_id = _escape_markup(dep["id"]) + if dep["reason"] == "missing": + console.print(f" [yellow]{extension_id}[/yellow] is not installed") + else: + console.print( + f" [yellow]{extension_id}[/yellow] " + f"{_escape_markup(dep['installed'])} does not satisfy " + f"{_escape_markup(dep['version'])}" + ) + console.print(f" Install with: specify extension add {extension_id}") + console.print() + console.print( + "[dim]The preset is installed and safe to use; the parts that rely on " + "these extensions will do nothing until they are present.[/dim]" + ) + + # ===== Preset Commands ===== @@ -283,6 +316,12 @@ def _validate_download_redirect(old_url, new_url): console.print("[red]Error:[/red] Specify a preset ID, --from URL, or --dev path") raise typer.Exit(1) + # Every install path above binds `manifest` and the no-source branch + # exits, so one call here covers --dev, --from, and catalog installs + # alike. Warns rather than fails: the preset is installed and its + # overrides fall through to the core workflow without the extension. + _warn_unmet_extension_dependencies(manager, manifest) + except PresetCompatibilityError as e: console.print(f"[red]Compatibility Error:[/red] {_escape_markup(str(e))}") raise typer.Exit(1) diff --git a/tests/test_presets.py b/tests/test_presets.py index 660a26d1b1..702c9c8778 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -535,6 +535,61 @@ def test_same_name_different_type_templates_allowed( manifest = PresetManifest(manifest_path) assert len(manifest.templates) == 2 + def test_requires_extensions_absent_is_valid(self, temp_dir, valid_pack_data): + """A preset with no declared dependencies stays valid and reports none.""" + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + assert PresetManifest(manifest_path).requires_extensions == [] + + def test_requires_extensions_accepts_both_forms(self, temp_dir, valid_pack_data): + """Bare ids and mappings normalize to the same shape.""" + valid_pack_data["requires"]["extensions"] = [ + "speckit-inventory", + {"id": "other-ext", "version": ">=1.2.0", "required": False}, + ] + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + + assert PresetManifest(manifest_path).requires_extensions == [ + {"id": "speckit-inventory", "version": None, "required": True}, + {"id": "other-ext", "version": ">=1.2.0", "required": False}, + ] + + @pytest.mark.parametrize( + "bad, expected", + [ + ("speckit-inventory", "Invalid requires.extensions"), # str, not list + ({"id": "x"}, "Invalid requires.extensions"), # mapping, not list + ([123], r"Invalid requires\.extensions\[0\]"), # member not str/mapping + ([None], r"Invalid requires\.extensions\[0\]"), + ([{"version": ">=1"}], r"Missing requires\.extensions\[0\]\.id"), + ([{"id": 5}], r"Invalid requires\.extensions\[0\]\.id"), + ([{"id": "Bad_ID"}], r"Invalid requires\.extensions\[0\]\.id"), + (["Bad_ID"], r"Invalid requires\.extensions\[0\]\.id"), + ([{"id": "x", "version": 1.0}], r"Invalid requires\.extensions\[0\]\.version"), + ([{"id": "x", "version": " "}], r"Invalid requires\.extensions\[0\]\.version"), + ([{"id": "x", "version": "nonsense"}], r"Invalid requires\.extensions\[0\]\.version"), + ([{"id": "x", "required": "yes"}], r"Invalid requires\.extensions\[0\]\.required"), + ], + ) + def test_requires_extensions_rejects_malformed( + self, temp_dir, valid_pack_data, bad, expected + ): + """Malformed dependency declarations fail as PresetValidationError. + + Same reasoning as requires.speckit_version: an unvalidated value reaches + ``SpecifierSet`` or ``re.match`` later and surfaces as a bare TypeError + that no caller handles as a malformed manifest. + """ + valid_pack_data["requires"]["extensions"] = bad + manifest_path = temp_dir / "preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + with pytest.raises(PresetValidationError, match=expected): + PresetManifest(manifest_path) + # ===== PresetRegistry Tests ===== @@ -1099,6 +1154,145 @@ def test_list_installed_includes_priority(self, project_dir, pack_dir): assert installed[0]["priority"] == 3 +class TestPresetExtensionDependencies: + """Test find_unmet_extension_dependencies (issue #4231).""" + + @staticmethod + def _install_extension(project_dir, extension_id, version): + """Register an installed extension the way the extension installer does.""" + extensions_dir = project_dir / ".specify" / "extensions" + extensions_dir.mkdir(parents=True, exist_ok=True) + registry_path = extensions_dir / ".registry" + data = {"schema_version": "1.0", "extensions": {}} + if registry_path.exists(): + data = json.loads(registry_path.read_text(encoding="utf-8")) + data["extensions"][extension_id] = {"version": version, "enabled": True} + registry_path.write_text(json.dumps(data), encoding="utf-8") + + @staticmethod + def _manifest(temp_dir, valid_pack_data, declared): + valid_pack_data["requires"]["extensions"] = declared + manifest_path = temp_dir / "dep-preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + return PresetManifest(manifest_path) + + def test_no_declared_dependencies_is_satisfied( + self, project_dir, temp_dir, valid_pack_data + ): + """A preset declaring nothing never reports an unmet dependency.""" + manifest_path = temp_dir / "plain-preset.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_pack_data, f) + + manager = PresetManager(project_dir) + assert manager.find_unmet_extension_dependencies( + PresetManifest(manifest_path) + ) == [] + + def test_missing_dependency_is_reported( + self, project_dir, temp_dir, valid_pack_data + ): + """An uninstalled required extension is reported as missing.""" + manifest = self._manifest(temp_dir, valid_pack_data, ["speckit-inventory"]) + + unmet = PresetManager(project_dir).find_unmet_extension_dependencies(manifest) + + assert len(unmet) == 1 + assert unmet[0]["id"] == "speckit-inventory" + assert unmet[0]["reason"] == "missing" + assert unmet[0]["installed"] is None + + def test_installed_dependency_is_satisfied( + self, project_dir, temp_dir, valid_pack_data + ): + """An installed extension with no version constraint is satisfied.""" + self._install_extension(project_dir, "speckit-inventory", "0.1.0") + manifest = self._manifest(temp_dir, valid_pack_data, ["speckit-inventory"]) + + assert PresetManager(project_dir).find_unmet_extension_dependencies( + manifest + ) == [] + + def test_satisfied_version_constraint( + self, project_dir, temp_dir, valid_pack_data + ): + """A satisfied version constraint reports nothing.""" + self._install_extension(project_dir, "speckit-inventory", "1.5.0") + manifest = self._manifest( + temp_dir, valid_pack_data, + [{"id": "speckit-inventory", "version": ">=1.2.0"}], + ) + + assert PresetManager(project_dir).find_unmet_extension_dependencies( + manifest + ) == [] + + def test_unsatisfied_version_constraint_reports_both_versions( + self, project_dir, temp_dir, valid_pack_data + ): + """A version mismatch reports the installed version alongside the constraint.""" + self._install_extension(project_dir, "speckit-inventory", "0.1.0") + manifest = self._manifest( + temp_dir, valid_pack_data, + [{"id": "speckit-inventory", "version": ">=9.0.0"}], + ) + + unmet = PresetManager(project_dir).find_unmet_extension_dependencies(manifest) + + assert len(unmet) == 1 + assert unmet[0]["reason"] == "version" + assert unmet[0]["installed"] == "0.1.0" + assert unmet[0]["version"] == ">=9.0.0" + + def test_optional_dependency_is_never_reported( + self, project_dir, temp_dir, valid_pack_data + ): + """`required: false` opts out of the warning even when absent.""" + manifest = self._manifest( + temp_dir, valid_pack_data, + [{"id": "speckit-inventory", "required": False}], + ) + + assert PresetManager(project_dir).find_unmet_extension_dependencies( + manifest + ) == [] + + def test_registry_entry_without_version_is_not_a_failure( + self, project_dir, temp_dir, valid_pack_data + ): + """An unusable registry version cannot be compared, so it is not invented + into a mismatch -- the extension is demonstrably installed.""" + self._install_extension(project_dir, "speckit-inventory", "0.1.0") + registry_path = project_dir / ".specify" / "extensions" / ".registry" + data = json.loads(registry_path.read_text(encoding="utf-8")) + data["extensions"]["speckit-inventory"]["version"] = None + registry_path.write_text(json.dumps(data), encoding="utf-8") + + manifest = self._manifest( + temp_dir, valid_pack_data, + [{"id": "speckit-inventory", "version": ">=9.0.0"}], + ) + + assert PresetManager(project_dir).find_unmet_extension_dependencies( + manifest + ) == [] + + def test_multiple_dependencies_report_independently( + self, project_dir, temp_dir, valid_pack_data + ): + """Each declared dependency is evaluated on its own.""" + self._install_extension(project_dir, "present-ext", "1.0.0") + manifest = self._manifest( + temp_dir, valid_pack_data, + ["present-ext", "absent-ext", {"id": "opt-ext", "required": False}], + ) + + unmet = PresetManager(project_dir).find_unmet_extension_dependencies(manifest) + + assert [dep["id"] for dep in unmet] == ["absent-ext"] + + class TestRegistryPriority: """Test registry priority sorting.""" From 853e67d97311bc3d5ac469c10163bbdd5ee3c211 Mon Sep 17 00:00:00 2001 From: Yash-Chindam Date: Fri, 21 Aug 2026 21:23:23 +0530 Subject: [PATCH 2/3] fix(presets): match dependency remediation to the reason, and flag disabled Addresses review feedback on #4250. `specify extension add ` refuses an already-installed extension without --force, so suggesting it for a version mismatch handed the user a command that could only fail. Suggest `extension update` for a version mismatch and `extension enable` for a disabled one, keeping `add` for a genuinely missing extension. A disabled extension was also treated as satisfied, because the registry entry exists. Resolution skips disabled extensions, so the preset stays exactly as inert as if the extension were absent, with no warning to explain it. Report it as a distinct "disabled" reason, ahead of any version check -- enabling is the prerequisite, and the version may be fine once it is. Also correct the closing line, which said the extensions "will do nothing until they are present" -- inaccurate for a disabled extension, which is present. Assisted-by: Claude Code (model: Claude Opus 5, autonomous) --- src/specify_cli/presets/__init__.py | 31 ++++++++++++++---- src/specify_cli/presets/_commands.py | 15 +++++++-- tests/test_presets.py | 49 +++++++++++++++++++++++++--- 3 files changed, 81 insertions(+), 14 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 302221ed8a..2006587bc0 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -969,8 +969,8 @@ def find_unmet_extension_dependencies( One entry per unsatisfied dependency, each with ``id``, the requested ``version`` specifier (``None`` when unconstrained), the ``installed`` version (``None`` when absent), and a ``reason`` of - either ``"missing"`` or ``"version"``. Optional dependencies - (``required: false``) are never reported. + ``"missing"``, ``"disabled"``, or ``"version"``. Optional + dependencies (``required: false``) are never reported. """ # Defense in depth, mirroring check_compatibility(): this method is # public and also reachable with a hand-built manifest object that @@ -995,18 +995,35 @@ def find_unmet_extension_dependencies( unmet.append({**dep, "installed": None, "reason": "missing"}) continue + installed_version = metadata.get("version") + installed_version = ( + installed_version if isinstance(installed_version, str) else None + ) + + # A disabled extension is registered but contributes nothing: + # resolution skips it (see _collect_extension_layers), so the + # preset is just as inert as if it were absent. Report it before + # any version check -- enabling it is the prerequisite, and the + # version may well be fine once it is. + if not metadata.get("enabled", True): + unmet.append( + {**dep, "installed": installed_version, "reason": "disabled"} + ) + continue + constraint = dep["version"] if not constraint: continue - installed = metadata.get("version") - if not isinstance(installed, str): + if installed_version is None: # A registry entry without a usable version cannot be compared. # Treat it as satisfied rather than inventing a failure, since - # the extension is demonstrably installed. + # the extension is demonstrably installed and enabled. continue - if not version_satisfies(installed, constraint): - unmet.append({**dep, "installed": installed, "reason": "version"}) + if not version_satisfies(installed_version, constraint): + unmet.append( + {**dep, "installed": installed_version, "reason": "version"} + ) return unmet diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index f7b02f14df..ece635800c 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -57,19 +57,28 @@ def _warn_unmet_extension_dependencies(manager, manifest) -> None: console.print("[yellow]![/yellow] This preset depends on extensions that are not satisfied:") for dep in unmet: extension_id = _escape_markup(dep["id"]) - if dep["reason"] == "missing": + reason = dep["reason"] + # The remediation has to match the reason. `extension add` refuses an + # already-installed extension without --force, so suggesting it for a + # disabled or out-of-date one would hand the user a command that fails. + if reason == "missing": console.print(f" [yellow]{extension_id}[/yellow] is not installed") + remedy = f"specify extension add {extension_id}" + elif reason == "disabled": + console.print(f" [yellow]{extension_id}[/yellow] is installed but disabled") + remedy = f"specify extension enable {extension_id}" else: console.print( f" [yellow]{extension_id}[/yellow] " f"{_escape_markup(dep['installed'])} does not satisfy " f"{_escape_markup(dep['version'])}" ) - console.print(f" Install with: specify extension add {extension_id}") + remedy = f"specify extension update {extension_id}" + console.print(f" Fix with: {remedy}") console.print() console.print( "[dim]The preset is installed and safe to use; the parts that rely on " - "these extensions will do nothing until they are present.[/dim]" + "these extensions will do nothing until this is resolved.[/dim]" ) diff --git a/tests/test_presets.py b/tests/test_presets.py index 702c9c8778..25875188fe 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1158,7 +1158,7 @@ class TestPresetExtensionDependencies: """Test find_unmet_extension_dependencies (issue #4231).""" @staticmethod - def _install_extension(project_dir, extension_id, version): + def _install_extension(project_dir, extension_id, version, enabled=True): """Register an installed extension the way the extension installer does.""" extensions_dir = project_dir / ".specify" / "extensions" extensions_dir.mkdir(parents=True, exist_ok=True) @@ -1166,7 +1166,7 @@ def _install_extension(project_dir, extension_id, version): data = {"schema_version": "1.0", "extensions": {}} if registry_path.exists(): data = json.loads(registry_path.read_text(encoding="utf-8")) - data["extensions"][extension_id] = {"version": version, "enabled": True} + data["extensions"][extension_id] = {"version": version, "enabled": enabled} registry_path.write_text(json.dumps(data), encoding="utf-8") @staticmethod @@ -1278,19 +1278,60 @@ def test_registry_entry_without_version_is_not_a_failure( manifest ) == [] + def test_disabled_dependency_is_reported( + self, project_dir, temp_dir, valid_pack_data + ): + """A disabled extension contributes nothing, so it counts as unmet. + + Resolution skips disabled extensions, leaving the preset just as inert + as if the extension were absent -- but the registry entry exists, so a + presence-only check would call it satisfied and stay silent. + """ + self._install_extension(project_dir, "speckit-inventory", "0.1.0", enabled=False) + manifest = self._manifest(temp_dir, valid_pack_data, ["speckit-inventory"]) + + unmet = PresetManager(project_dir).find_unmet_extension_dependencies(manifest) + + assert len(unmet) == 1 + assert unmet[0]["reason"] == "disabled" + assert unmet[0]["installed"] == "0.1.0" + + def test_disabled_is_reported_ahead_of_version_mismatch( + self, project_dir, temp_dir, valid_pack_data + ): + """Enabling is the prerequisite, so it is reported before the version.""" + self._install_extension(project_dir, "speckit-inventory", "0.1.0", enabled=False) + manifest = self._manifest( + temp_dir, valid_pack_data, + [{"id": "speckit-inventory", "version": ">=9.0.0"}], + ) + + unmet = PresetManager(project_dir).find_unmet_extension_dependencies(manifest) + + assert [dep["reason"] for dep in unmet] == ["disabled"] + def test_multiple_dependencies_report_independently( self, project_dir, temp_dir, valid_pack_data ): """Each declared dependency is evaluated on its own.""" self._install_extension(project_dir, "present-ext", "1.0.0") + self._install_extension(project_dir, "off-ext", "1.0.0", enabled=False) manifest = self._manifest( temp_dir, valid_pack_data, - ["present-ext", "absent-ext", {"id": "opt-ext", "required": False}], + [ + "present-ext", + "absent-ext", + "off-ext", + {"id": "opt-ext", "required": False}, + ], ) unmet = PresetManager(project_dir).find_unmet_extension_dependencies(manifest) - assert [dep["id"] for dep in unmet] == ["absent-ext"] + assert [(dep["id"], dep["reason"]) for dep in unmet] == [ + ("absent-ext", "missing"), + ("off-ext", "disabled"), + ] class TestRegistryPriority: From 972cb57bbfdd4742a2c07a63e2ad9ad97be0ef91 Mon Sep 17 00:00:00 2001 From: Yash-Chindam Date: Fri, 21 Aug 2026 22:42:47 +0530 Subject: [PATCH 3/3] fix(presets): avoid promising unsatisfiable extension updates Assisted-by: ChatGPT (model: GPT-5, supervised) --- src/specify_cli/presets/_commands.py | 11 ++++++++--- tests/test_presets.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/presets/_commands.py b/src/specify_cli/presets/_commands.py index ece635800c..5594273c7a 100644 --- a/src/specify_cli/presets/_commands.py +++ b/src/specify_cli/presets/_commands.py @@ -59,8 +59,10 @@ def _warn_unmet_extension_dependencies(manager, manifest) -> None: extension_id = _escape_markup(dep["id"]) reason = dep["reason"] # The remediation has to match the reason. `extension add` refuses an - # already-installed extension without --force, so suggesting it for a - # disabled or out-of-date one would hand the user a command that fails. + # already-installed extension without --force, and `extension update` + # only moves forward to the catalog release. A general PEP 440 + # constraint may require an exact version, an upper bound, or a + # downgrade, so do not promise that update will satisfy it. if reason == "missing": console.print(f" [yellow]{extension_id}[/yellow] is not installed") remedy = f"specify extension add {extension_id}" @@ -73,7 +75,10 @@ def _warn_unmet_extension_dependencies(manager, manifest) -> None: f"{_escape_markup(dep['installed'])} does not satisfy " f"{_escape_markup(dep['version'])}" ) - remedy = f"specify extension update {extension_id}" + remedy = ( + "install a release of " + f"{extension_id} satisfying {_escape_markup(dep['version'])}" + ) console.print(f" Fix with: {remedy}") console.print() console.print( diff --git a/tests/test_presets.py b/tests/test_presets.py index 25875188fe..c1e85b691e 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -40,6 +40,8 @@ VALID_PRESET_TEMPLATE_TYPES, ) from specify_cli.extensions import ExtensionRegistry +from specify_cli._console import console +from specify_cli.presets._commands import _warn_unmet_extension_dependencies # ===== Fixtures ===== @@ -1245,6 +1247,25 @@ def test_unsatisfied_version_constraint_reports_both_versions( assert unmet[0]["installed"] == "0.1.0" assert unmet[0]["version"] == ">=9.0.0" + def test_version_warning_does_not_promise_update_satisfies_constraint(self): + """Version remediation must handle constraints update cannot guarantee.""" + manager = MagicMock() + manager.find_unmet_extension_dependencies.return_value = [ + { + "id": "speckit-inventory", + "reason": "version", + "installed": "3.0.0", + "version": "<2", + } + ] + + with console.capture() as capture: + _warn_unmet_extension_dependencies(manager, MagicMock()) + + output = strip_ansi(capture.get()) + assert "install a release of speckit-inventory satisfying <2" in output + assert "specify extension update" not in output + def test_optional_dependency_is_never_reported( self, project_dir, temp_dir, valid_pack_data ):