Skip to content

Commit 3fed7f7

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 16f03ff commit 3fed7f7

File tree

2 files changed

+110
-30
lines changed

2 files changed

+110
-30
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: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -303,8 +303,23 @@ def test_command_name_mismatched_namespace_not_corrected(self, temp_dir, valid_m
303303
with pytest.raises(ValidationError, match="Invalid command name"):
304304
ExtensionManifest(manifest_path)
305305

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

310325
valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.hello"]
@@ -315,10 +330,29 @@ def test_alias_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_data):
315330

316331
manifest = ExtensionManifest(manifest_path)
317332

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

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

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

721755
ext_dir = temp_dir / "alias-shortcut"
@@ -748,10 +782,10 @@ def test_install_autocorrects_alias_without_extension_namespace(self, temp_dir,
748782
manager = ExtensionManager(project_dir)
749783
manifest = manager.install_from_directory(ext_dir, "0.1.0", register_commands=False)
750784

751-
assert manifest.commands[0]["aliases"] == ["speckit.alias-shortcut.shortcut"]
785+
assert manifest.commands[0]["aliases"] == ["alias-shortcut.shortcut"]
752786
assert len(manifest.warnings) == 1
753787
assert "speckit.shortcut" in manifest.warnings[0]
754-
assert "speckit.alias-shortcut.shortcut" in manifest.warnings[0]
788+
assert "alias-shortcut.shortcut" in manifest.warnings[0]
755789

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

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

795835
first_dir = temp_dir / "ext-one"
796836
first_dir.mkdir()
797837
(first_dir / "commands").mkdir()
838+
# Directly write a manifest that claims speckit.shared.sync even though its id is
839+
# "ext-one" — this bypasses install-time namespace enforcement and simulates a
840+
# legacy/corrupted extension that already occupies the name.
798841
first_manifest = {
799842
"schema_version": "1.0",
800843
"extension": {
@@ -807,9 +850,8 @@ def test_install_rejects_command_collision_with_installed_extension(self, temp_d
807850
"provides": {
808851
"commands": [
809852
{
810-
"name": "speckit.ext-one.sync",
853+
"name": "speckit.shared.sync",
811854
"file": "commands/cmd.md",
812-
"aliases": ["speckit.shared.sync"],
813855
}
814856
]
815857
},
@@ -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,9 +1211,9 @@ 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.md").exists()
1174-
assert (claude_dir / "speckit.ext-alias.shortcut.md").exists()
1216+
assert (claude_dir / "ext-alias.shortcut.md").exists()
11751217

11761218
def test_unregister_commands_for_codex_skills_uses_mapped_names(self, project_dir):
11771219
"""Codex skill cleanup should use the same mapped names as registration."""
@@ -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)