Skip to content

Commit caee3e0

Browse files
iamaeroplaneclaude
andcommitted
fix(tests): fix 4 failing tests and normalize null aliases in manifest validation (#2017)
- Remove accidental hook reference from alias auto-correction test (fixture has after_tasks hook whose command matches the alias being renamed, causing a spurious second warning) - Update collision test fixture to use a primary command collision instead of an alias-based one (alias namespace enforcement now makes cross-extension alias collisions impossible by design) - Update test_command_with_aliases and test_copilot_aliases_get_companion_prompts to use the new 2-part alias format (ext-id.command) rather than the legacy 3-part speckit.ext.command form - Normalize aliases: null to aliases: [] in _validate() so downstream cmd_info.get('aliases', []) cannot return None when the YAML key exists but is explicitly null Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 01bb599 commit caee3e0

File tree

2 files changed

+109
-29
lines changed

2 files changed

+109
-29
lines changed

src/specify_cli/extensions.py

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@
3737
"taskstoissues",
3838
})
3939
EXTENSION_COMMAND_NAME_PATTERN = re.compile(r"^speckit\.([a-z0-9-]+)\.([a-z0-9-]+)$")
40+
# Aliases use a 2-part 'namespace.command' form without the 'speckit.' prefix.
41+
# The 'speckit' namespace is reserved for core commands.
42+
EXTENSION_ALIAS_PATTERN = re.compile(r"^([a-z0-9-]+)\.([a-z0-9-]+)$")
4043

4144

4245
def _load_core_command_names() -> frozenset[str]:
@@ -214,6 +217,7 @@ def _validate(self):
214217
aliases = cmd.get("aliases")
215218
if aliases is None:
216219
aliases = []
220+
cmd["aliases"] = [] # normalize null/missing to empty list in-place
217221
if not isinstance(aliases, list):
218222
raise ValidationError(
219223
f"Aliases for command '{cmd['name']}' must be a list"
@@ -223,20 +227,24 @@ def _validate(self):
223227
raise ValidationError(
224228
f"Aliases for command '{cmd['name']}' must be strings"
225229
)
226-
if not EXTENSION_COMMAND_NAME_PATTERN.match(alias):
227-
corrected = self._try_correct_command_name(alias, ext["id"])
230+
alias_match = EXTENSION_ALIAS_PATTERN.match(alias)
231+
if alias_match and alias_match.group(1) != 'speckit':
232+
pass # already valid: 'myext.command' form
233+
else:
234+
corrected = self._try_correct_alias_name(alias, ext["id"])
228235
if corrected:
229236
self.warnings.append(
230237
f"Alias '{alias}' does not follow the required pattern "
231-
f"'speckit.{{extension}}.{{command}}'. Registering as '{corrected}'. "
238+
f"'{{extension}}.{{command}}'. Registering as '{corrected}'. "
232239
f"The extension author should update the manifest to use this name."
233240
)
234241
rename_map[alias] = corrected
235242
aliases[i] = corrected
236243
else:
237244
raise ValidationError(
238245
f"Invalid alias '{alias}': "
239-
"must follow pattern 'speckit.{extension}.{command}'"
246+
"must follow pattern '{extension}.{command}' "
247+
"(the 'speckit.' prefix is reserved for core commands)"
240248
)
241249

242250
# Rewrite any hook command references that pointed at a renamed command.
@@ -252,15 +260,14 @@ def _validate(self):
252260

253261
@staticmethod
254262
def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]:
255-
"""Try to auto-correct a non-conforming command name to the required pattern.
263+
"""Try to auto-correct a non-conforming primary command name to 'speckit.{ext_id}.command'.
256264
257-
Handles the two legacy formats used by community extensions:
265+
Handles legacy formats:
258266
- 'speckit.command' → 'speckit.{ext_id}.command'
259267
- '{ext_id}.command' → 'speckit.{ext_id}.command'
260268
261269
The 'X.Y' form is only corrected when X matches ext_id to ensure the
262-
result passes the install-time namespace check. Any other prefix is
263-
uncorrectable and will produce a ValidationError at the call site.
270+
result passes the install-time namespace check.
264271
265272
Returns the corrected name, or None if no safe correction is possible.
266273
"""
@@ -272,6 +279,27 @@ def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]:
272279
return candidate
273280
return None
274281

282+
@staticmethod
283+
def _try_correct_alias_name(name: str, ext_id: str) -> Optional[str]:
284+
"""Try to auto-correct a non-conforming alias to the '{ext_id}.command' pattern.
285+
286+
Handles legacy formats:
287+
- 'speckit.command' → '{ext_id}.command'
288+
- 'speckit.ext_id.command' → 'ext_id.command' (3-part with speckit prefix)
289+
290+
Returns the corrected name, or None if no safe correction is possible.
291+
"""
292+
parts = name.split('.')
293+
if len(parts) == 2 and parts[0] == 'speckit':
294+
candidate = f"{ext_id}.{parts[1]}"
295+
if EXTENSION_ALIAS_PATTERN.match(candidate):
296+
return candidate
297+
if len(parts) == 3 and parts[0] == 'speckit':
298+
candidate = f"{parts[1]}.{parts[2]}"
299+
if EXTENSION_ALIAS_PATTERN.match(candidate):
300+
return candidate
301+
return None
302+
275303
@property
276304
def id(self) -> str:
277305
"""Get extension ID."""
@@ -608,14 +636,24 @@ def _collect_manifest_command_names(manifest: ExtensionManifest) -> Dict[str, st
608636
f"{kind.capitalize()} for command '{primary_name}' must be a string"
609637
)
610638

611-
match = EXTENSION_COMMAND_NAME_PATTERN.match(name)
612-
if match is None:
613-
raise ValidationError(
614-
f"Invalid {kind} '{name}': "
615-
"must follow pattern 'speckit.{extension}.{command}'"
616-
)
639+
if kind == "command":
640+
match = EXTENSION_COMMAND_NAME_PATTERN.match(name)
641+
if match is None:
642+
raise ValidationError(
643+
f"Invalid command '{name}': "
644+
"must follow pattern 'speckit.{extension}.{command}'"
645+
)
646+
namespace = match.group(1)
647+
else:
648+
match = EXTENSION_ALIAS_PATTERN.match(name)
649+
if match is None or match.group(1) == 'speckit':
650+
raise ValidationError(
651+
f"Invalid alias '{name}': "
652+
"must follow pattern '{extension}.{command}' "
653+
"(the 'speckit.' prefix is reserved for core commands)"
654+
)
655+
namespace = match.group(1)
617656

618-
namespace = match.group(1)
619657
if namespace != manifest.id:
620658
raise ValidationError(
621659
f"{kind.capitalize()} '{name}' must use extension namespace '{manifest.id}'"

tests/test_extensions.py

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -304,8 +304,23 @@ def test_command_name_mismatched_namespace_not_corrected(self, temp_dir, valid_m
304304
with pytest.raises(ValidationError, match="Invalid command name"):
305305
ExtensionManifest(manifest_path)
306306

307-
def test_alias_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_data):
308-
"""Test that a legacy 'speckit.command' alias is auto-corrected."""
307+
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."""
309+
import yaml
310+
311+
valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.hello"]
312+
313+
manifest_path = temp_dir / "extension.yml"
314+
with open(manifest_path, 'w') as f:
315+
yaml.dump(valid_manifest_data, f)
316+
317+
manifest = ExtensionManifest(manifest_path)
318+
319+
assert manifest.commands[0]["aliases"] == ["test-ext.hello"]
320+
assert manifest.warnings == []
321+
322+
def test_alias_autocorrect_speckit_two_part(self, temp_dir, valid_manifest_data):
323+
"""Test that legacy 'speckit.command' alias is corrected to '{ext_id}.command'."""
309324
import yaml
310325

311326
valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.hello"]
@@ -316,10 +331,29 @@ def test_alias_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_data):
316331

317332
manifest = ExtensionManifest(manifest_path)
318333

319-
assert manifest.commands[0]["aliases"] == ["speckit.test-ext.hello"]
334+
assert manifest.commands[0]["aliases"] == ["test-ext.hello"]
320335
assert len(manifest.warnings) == 1
321336
assert "speckit.hello" in manifest.warnings[0]
337+
assert "test-ext.hello" in manifest.warnings[0]
338+
339+
def test_alias_autocorrect_speckit_three_part(self, temp_dir, valid_manifest_data):
340+
"""Test that a 3-part 'speckit.ext.command' alias is corrected to 'ext.command'."""
341+
import yaml
342+
343+
# Clear hooks so the alias rename doesn't also trigger a hook-reference warning.
344+
valid_manifest_data.pop("hooks", None)
345+
valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.test-ext.hello"]
346+
347+
manifest_path = temp_dir / "extension.yml"
348+
with open(manifest_path, 'w') as f:
349+
yaml.dump(valid_manifest_data, f)
350+
351+
manifest = ExtensionManifest(manifest_path)
352+
353+
assert manifest.commands[0]["aliases"] == ["test-ext.hello"]
354+
assert len(manifest.warnings) == 1
322355
assert "speckit.test-ext.hello" in manifest.warnings[0]
356+
assert "test-ext.hello" in manifest.warnings[0]
323357

324358
def test_valid_command_name_has_no_warnings(self, temp_dir, valid_manifest_data):
325359
"""Test that a correctly-named command produces no warnings."""
@@ -715,8 +749,8 @@ def test_install_rejects_extension_id_in_core_namespace(self, temp_dir, project_
715749
with pytest.raises(ValidationError, match="conflicts with core command namespace"):
716750
manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
717751

718-
def test_install_autocorrects_alias_without_extension_namespace(self, temp_dir, project_dir):
719-
"""Legacy short aliases are auto-corrected to 'speckit.{ext_id}.{cmd}' with a warning."""
752+
def test_install_autocorrects_speckit_alias_to_ext_form(self, temp_dir, project_dir):
753+
"""Legacy 'speckit.command' aliases are corrected to '{ext_id}.command' with a warning."""
720754
import yaml
721755

722756
ext_dir = temp_dir / "alias-shortcut"
@@ -749,10 +783,10 @@ def test_install_autocorrects_alias_without_extension_namespace(self, temp_dir,
749783
manager = ExtensionManager(project_dir)
750784
manifest = manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
751785

752-
assert manifest.commands[0]["aliases"] == ["speckit.alias-shortcut.shortcut"]
786+
assert manifest.commands[0]["aliases"] == ["alias-shortcut.shortcut"]
753787
assert len(manifest.warnings) == 1
754788
assert "speckit.shortcut" in manifest.warnings[0]
755-
assert "speckit.alias-shortcut.shortcut" in manifest.warnings[0]
789+
assert "alias-shortcut.shortcut" in manifest.warnings[0]
756790

757791
def test_install_rejects_namespace_squatting(self, temp_dir, project_dir):
758792
"""Install should reject commands and aliases outside the extension namespace."""
@@ -790,12 +824,21 @@ def test_install_rejects_namespace_squatting(self, temp_dir, project_dir):
790824
manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
791825

792826
def test_install_rejects_command_collision_with_installed_extension(self, temp_dir, project_dir):
793-
"""Install should reject names already claimed by an installed legacy extension."""
827+
"""Install should reject names already claimed by an installed extension.
828+
829+
The first extension is written directly to disk (bypassing install_from_directory) to
830+
simulate a scenario where it claims a command in a different namespace — something that
831+
could happen with a legacy or manually-placed extension. The collision detector must
832+
still catch the conflict when the second extension tries to register the same command.
833+
"""
794834
import yaml
795835

796836
first_dir = temp_dir / "ext-one"
797837
first_dir.mkdir()
798838
(first_dir / "commands").mkdir()
839+
# Directly write a manifest that claims speckit.shared.sync even though its id is
840+
# "ext-one" — this bypasses install-time namespace enforcement and simulates a
841+
# legacy/corrupted extension that already occupies the name.
799842
first_manifest = {
800843
"schema_version": "1.0",
801844
"extension": {
@@ -808,9 +851,8 @@ def test_install_rejects_command_collision_with_installed_extension(self, temp_d
808851
"provides": {
809852
"commands": [
810853
{
811-
"name": "speckit.ext-one.sync",
854+
"name": "speckit.shared.sync",
812855
"file": "commands/cmd.md",
813-
"aliases": ["speckit.shared.sync"],
814856
}
815857
]
816858
},
@@ -1148,7 +1190,7 @@ def test_command_with_aliases(self, project_dir, temp_dir):
11481190
{
11491191
"name": "speckit.ext-alias.cmd",
11501192
"file": "commands/cmd.md",
1151-
"aliases": ["speckit.ext-alias.shortcut"],
1193+
"aliases": ["ext-alias.shortcut"],
11521194
}
11531195
]
11541196
},
@@ -1169,7 +1211,7 @@ def test_command_with_aliases(self, project_dir, temp_dir):
11691211

11701212
assert len(registered) == 2
11711213
assert "speckit.ext-alias.cmd" in registered
1172-
assert "speckit.ext-alias.shortcut" in registered
1214+
assert "ext-alias.shortcut" in registered
11731215
assert (claude_dir / "speckit-ext-alias-cmd" / "SKILL.md").exists()
11741216
assert (claude_dir / "speckit-ext-alias-shortcut" / "SKILL.md").exists()
11751217

@@ -1591,7 +1633,7 @@ def test_copilot_aliases_get_companion_prompts(self, project_dir, temp_dir):
15911633
{
15921634
"name": "speckit.ext-alias-copilot.cmd",
15931635
"file": "commands/cmd.md",
1594-
"aliases": ["speckit.ext-alias-copilot.shortcut"],
1636+
"aliases": ["ext-alias-copilot.shortcut"],
15951637
}
15961638
]
15971639
},
@@ -1619,7 +1661,7 @@ def test_copilot_aliases_get_companion_prompts(self, project_dir, temp_dir):
16191661
# Both primary and alias get companion .prompt.md
16201662
prompts_dir = project_dir / ".github" / "prompts"
16211663
assert (prompts_dir / "speckit.ext-alias-copilot.cmd.prompt.md").exists()
1622-
assert (prompts_dir / "speckit.ext-alias-copilot.shortcut.prompt.md").exists()
1664+
assert (prompts_dir / "ext-alias-copilot.shortcut.prompt.md").exists()
16231665

16241666
def test_non_copilot_agent_no_companion_file(self, extension_dir, project_dir):
16251667
"""Test that non-copilot agents do NOT create .prompt.md files."""

0 commit comments

Comments
 (0)