Skip to content

Commit 853e67d

Browse files
committed
fix(presets): match dependency remediation to the reason, and flag disabled
Addresses review feedback on #4250. `specify extension add <id>` 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)
1 parent b674156 commit 853e67d

3 files changed

Lines changed: 81 additions & 14 deletions

File tree

‎src/specify_cli/presets/__init__.py‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -969,8 +969,8 @@ def find_unmet_extension_dependencies(
969969
One entry per unsatisfied dependency, each with ``id``, the
970970
requested ``version`` specifier (``None`` when unconstrained), the
971971
``installed`` version (``None`` when absent), and a ``reason`` of
972-
either ``"missing"`` or ``"version"``. Optional dependencies
973-
(``required: false``) are never reported.
972+
``"missing"``, ``"disabled"``, or ``"version"``. Optional
973+
dependencies (``required: false``) are never reported.
974974
"""
975975
# Defense in depth, mirroring check_compatibility(): this method is
976976
# public and also reachable with a hand-built manifest object that
@@ -995,18 +995,35 @@ def find_unmet_extension_dependencies(
995995
unmet.append({**dep, "installed": None, "reason": "missing"})
996996
continue
997997

998+
installed_version = metadata.get("version")
999+
installed_version = (
1000+
installed_version if isinstance(installed_version, str) else None
1001+
)
1002+
1003+
# A disabled extension is registered but contributes nothing:
1004+
# resolution skips it (see _collect_extension_layers), so the
1005+
# preset is just as inert as if it were absent. Report it before
1006+
# any version check -- enabling it is the prerequisite, and the
1007+
# version may well be fine once it is.
1008+
if not metadata.get("enabled", True):
1009+
unmet.append(
1010+
{**dep, "installed": installed_version, "reason": "disabled"}
1011+
)
1012+
continue
1013+
9981014
constraint = dep["version"]
9991015
if not constraint:
10001016
continue
10011017

1002-
installed = metadata.get("version")
1003-
if not isinstance(installed, str):
1018+
if installed_version is None:
10041019
# A registry entry without a usable version cannot be compared.
10051020
# Treat it as satisfied rather than inventing a failure, since
1006-
# the extension is demonstrably installed.
1021+
# the extension is demonstrably installed and enabled.
10071022
continue
1008-
if not version_satisfies(installed, constraint):
1009-
unmet.append({**dep, "installed": installed, "reason": "version"})
1023+
if not version_satisfies(installed_version, constraint):
1024+
unmet.append(
1025+
{**dep, "installed": installed_version, "reason": "version"}
1026+
)
10101027

10111028
return unmet
10121029

‎src/specify_cli/presets/_commands.py‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,19 +57,28 @@ def _warn_unmet_extension_dependencies(manager, manifest) -> None:
5757
console.print("[yellow]![/yellow] This preset depends on extensions that are not satisfied:")
5858
for dep in unmet:
5959
extension_id = _escape_markup(dep["id"])
60-
if dep["reason"] == "missing":
60+
reason = dep["reason"]
61+
# The remediation has to match the reason. `extension add` refuses an
62+
# already-installed extension without --force, so suggesting it for a
63+
# disabled or out-of-date one would hand the user a command that fails.
64+
if reason == "missing":
6165
console.print(f" [yellow]{extension_id}[/yellow] is not installed")
66+
remedy = f"specify extension add {extension_id}"
67+
elif reason == "disabled":
68+
console.print(f" [yellow]{extension_id}[/yellow] is installed but disabled")
69+
remedy = f"specify extension enable {extension_id}"
6270
else:
6371
console.print(
6472
f" [yellow]{extension_id}[/yellow] "
6573
f"{_escape_markup(dep['installed'])} does not satisfy "
6674
f"{_escape_markup(dep['version'])}"
6775
)
68-
console.print(f" Install with: specify extension add {extension_id}")
76+
remedy = f"specify extension update {extension_id}"
77+
console.print(f" Fix with: {remedy}")
6978
console.print()
7079
console.print(
7180
"[dim]The preset is installed and safe to use; the parts that rely on "
72-
"these extensions will do nothing until they are present.[/dim]"
81+
"these extensions will do nothing until this is resolved.[/dim]"
7382
)
7483

7584

‎tests/test_presets.py‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1158,15 +1158,15 @@ class TestPresetExtensionDependencies:
11581158
"""Test find_unmet_extension_dependencies (issue #4231)."""
11591159

11601160
@staticmethod
1161-
def _install_extension(project_dir, extension_id, version):
1161+
def _install_extension(project_dir, extension_id, version, enabled=True):
11621162
"""Register an installed extension the way the extension installer does."""
11631163
extensions_dir = project_dir / ".specify" / "extensions"
11641164
extensions_dir.mkdir(parents=True, exist_ok=True)
11651165
registry_path = extensions_dir / ".registry"
11661166
data = {"schema_version": "1.0", "extensions": {}}
11671167
if registry_path.exists():
11681168
data = json.loads(registry_path.read_text(encoding="utf-8"))
1169-
data["extensions"][extension_id] = {"version": version, "enabled": True}
1169+
data["extensions"][extension_id] = {"version": version, "enabled": enabled}
11701170
registry_path.write_text(json.dumps(data), encoding="utf-8")
11711171

11721172
@staticmethod
@@ -1278,19 +1278,60 @@ def test_registry_entry_without_version_is_not_a_failure(
12781278
manifest
12791279
) == []
12801280

1281+
def test_disabled_dependency_is_reported(
1282+
self, project_dir, temp_dir, valid_pack_data
1283+
):
1284+
"""A disabled extension contributes nothing, so it counts as unmet.
1285+
1286+
Resolution skips disabled extensions, leaving the preset just as inert
1287+
as if the extension were absent -- but the registry entry exists, so a
1288+
presence-only check would call it satisfied and stay silent.
1289+
"""
1290+
self._install_extension(project_dir, "speckit-inventory", "0.1.0", enabled=False)
1291+
manifest = self._manifest(temp_dir, valid_pack_data, ["speckit-inventory"])
1292+
1293+
unmet = PresetManager(project_dir).find_unmet_extension_dependencies(manifest)
1294+
1295+
assert len(unmet) == 1
1296+
assert unmet[0]["reason"] == "disabled"
1297+
assert unmet[0]["installed"] == "0.1.0"
1298+
1299+
def test_disabled_is_reported_ahead_of_version_mismatch(
1300+
self, project_dir, temp_dir, valid_pack_data
1301+
):
1302+
"""Enabling is the prerequisite, so it is reported before the version."""
1303+
self._install_extension(project_dir, "speckit-inventory", "0.1.0", enabled=False)
1304+
manifest = self._manifest(
1305+
temp_dir, valid_pack_data,
1306+
[{"id": "speckit-inventory", "version": ">=9.0.0"}],
1307+
)
1308+
1309+
unmet = PresetManager(project_dir).find_unmet_extension_dependencies(manifest)
1310+
1311+
assert [dep["reason"] for dep in unmet] == ["disabled"]
1312+
12811313
def test_multiple_dependencies_report_independently(
12821314
self, project_dir, temp_dir, valid_pack_data
12831315
):
12841316
"""Each declared dependency is evaluated on its own."""
12851317
self._install_extension(project_dir, "present-ext", "1.0.0")
1318+
self._install_extension(project_dir, "off-ext", "1.0.0", enabled=False)
12861319
manifest = self._manifest(
12871320
temp_dir, valid_pack_data,
1288-
["present-ext", "absent-ext", {"id": "opt-ext", "required": False}],
1321+
[
1322+
"present-ext",
1323+
"absent-ext",
1324+
"off-ext",
1325+
{"id": "opt-ext", "required": False},
1326+
],
12891327
)
12901328

12911329
unmet = PresetManager(project_dir).find_unmet_extension_dependencies(manifest)
12921330

1293-
assert [dep["id"] for dep in unmet] == ["absent-ext"]
1331+
assert [(dep["id"], dep["reason"]) for dep in unmet] == [
1332+
("absent-ext", "missing"),
1333+
("off-ext", "disabled"),
1334+
]
12941335

12951336

12961337
class TestRegistryPriority:

0 commit comments

Comments
 (0)