Skip to content

Commit 5a152ea

Browse files
iamaeroplaneclaude
andcommitted
fix(extensions): canonicalize hook alias refs and fix docstrings (#2017)
- Hook refs in alias form (ext.cmd) are now lifted to canonical speckit.{ext}.cmd so _skill_name_from_command maps them correctly in skill-mode invocation; non-dict hook entries are skipped safely - Validate that 'hooks' is a mapping; raise ValidationError for non-dict hooks value instead of AttributeError - Update _collect_manifest_command_names docstring to reflect that aliases now use {extension}.{command}, not speckit.{extension}.{command} - Fix test docstring to name the concrete alias value used in the test - Add tests for hook alias canonicalization and non-dict hooks validation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 240f715 commit 5a152ea

2 files changed

Lines changed: 57 additions & 8 deletions

File tree

src/specify_cli/extensions.py

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,21 +247,39 @@ def _validate(self):
247247
"(the 'speckit.' prefix is reserved for core commands)"
248248
)
249249

250-
# Rewrite any hook command references that pointed at a renamed command.
250+
# Rewrite any hook command references that pointed at a renamed command,
251+
# then canonicalize alias-form refs (e.g. 'ext.cmd') to the canonical
252+
# 'speckit.{ext}.cmd' form so hook-invocation skill-name mapping works.
251253
hooks = self.data.get("hooks", {})
252254
if not isinstance(hooks, dict):
253255
raise ValidationError(
254256
f"'hooks' must be a mapping, got {type(hooks).__name__}"
255257
)
256258
for hook_name, hook_data in hooks.items():
257-
if isinstance(hook_data, dict) and hook_data.get("command") in rename_map:
258-
old_ref = hook_data["command"]
259-
hook_data["command"] = rename_map[old_ref]
259+
if not isinstance(hook_data, dict):
260+
continue
261+
command_ref = hook_data.get("command")
262+
if not isinstance(command_ref, str):
263+
continue
264+
265+
# Step 1: apply rename_map (command was auto-corrected during validation).
266+
rewritten = rename_map.get(command_ref, command_ref)
267+
if rewritten != command_ref:
268+
hook_data["command"] = rewritten
260269
self.warnings.append(
261-
f"Hook '{hook_name}' referenced renamed command '{old_ref}'; "
262-
f"updated to '{rename_map[old_ref]}'. "
270+
f"Hook '{hook_name}' referenced renamed command '{command_ref}'; "
271+
f"updated to '{rewritten}'. "
263272
f"The extension author should update the manifest."
264273
)
274+
command_ref = rewritten
275+
276+
# Step 2: if the ref is in alias form '{ext_id}.cmd', lift it to
277+
# 'speckit.{ext_id}.cmd' so skill-mode hook invocation renders correctly.
278+
parts = command_ref.split('.')
279+
if len(parts) == 2 and parts[0] == ext["id"]:
280+
canonical = f"speckit.{ext['id']}.{parts[1]}"
281+
if EXTENSION_COMMAND_NAME_PATTERN.match(canonical):
282+
hook_data["command"] = canonical
265283

266284
@staticmethod
267285
def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]:
@@ -601,7 +619,8 @@ def _collect_manifest_command_names(manifest: ExtensionManifest) -> Dict[str, st
601619
"""Collect command and alias names declared by a manifest.
602620
603621
Performs install-time validation for extension-specific constraints:
604-
- commands and aliases must use the canonical `speckit.{extension}.{command}` shape
622+
- primary commands must use the canonical `speckit.{extension}.{command}` shape
623+
- aliases must use the `{extension}.{command}` shape (no `speckit.` prefix)
605624
- commands and aliases must use this extension's namespace
606625
- command namespaces must not shadow core commands
607626
- duplicate command/alias names inside one manifest are rejected

tests/test_extensions.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ def test_command_name_mismatched_namespace_not_corrected(self, temp_dir, valid_m
305305
ExtensionManifest(manifest_path)
306306

307307
def test_alias_valid_two_part_no_prefix(self, temp_dir, valid_manifest_data):
308-
"""Test that a 'myext.command' alias is accepted as-is with no warning."""
308+
"""Test that a 'test-ext.hello' alias is accepted as-is with no warning."""
309309
import yaml
310310

311311
valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.hello"]
@@ -355,6 +355,36 @@ def test_alias_autocorrect_speckit_three_part(self, temp_dir, valid_manifest_dat
355355
assert "speckit.test-ext.hello" in manifest.warnings[0]
356356
assert "test-ext.hello" in manifest.warnings[0]
357357

358+
def test_hook_alias_ref_canonicalized_to_speckit_form(self, temp_dir, valid_manifest_data):
359+
"""Hook command refs in alias form are lifted to canonical speckit.ext.cmd form."""
360+
import yaml
361+
362+
# Manifest uses a valid alias but the hook mistakenly references the alias name.
363+
valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.hello"]
364+
valid_manifest_data["hooks"]["after_tasks"]["command"] = "test-ext.hello"
365+
366+
manifest_path = temp_dir / "extension.yml"
367+
with open(manifest_path, "w") as f:
368+
yaml.dump(valid_manifest_data, f)
369+
370+
manifest = ExtensionManifest(manifest_path)
371+
372+
# Hook ref should be lifted to canonical form for skill-mode invocation.
373+
assert manifest.hooks["after_tasks"]["command"] == "speckit.test-ext.hello"
374+
375+
def test_hook_non_dict_value_raises(self, temp_dir, valid_manifest_data):
376+
"""A non-mapping hooks value raises ValidationError."""
377+
import yaml
378+
379+
valid_manifest_data["hooks"] = ["not", "a", "dict"]
380+
381+
manifest_path = temp_dir / "extension.yml"
382+
with open(manifest_path, "w") as f:
383+
yaml.dump(valid_manifest_data, f)
384+
385+
with pytest.raises(ValidationError, match="'hooks' must be a mapping"):
386+
ExtensionManifest(manifest_path)
387+
358388
def test_valid_command_name_has_no_warnings(self, temp_dir, valid_manifest_data):
359389
"""Test that a correctly-named command produces no warnings."""
360390
import yaml

0 commit comments

Comments
 (0)