From 72e2a53298e34c72cd3177a0a0627894c5751de1 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Mon, 30 Mar 2026 21:51:03 +0200 Subject: [PATCH 01/16] docs: warn about unofficial PyPI packages and recommend version verification (#1982) Clarify that only packages from github/spec-kit are official, and add `specify version` as a post-install verification step to help users catch accidental installation of an unrelated package with the same name. Co-Authored-By: Claude Sonnet 4.6 --- README.md | 10 +++++++++- docs/installation.md | 10 ++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ab14ace731..dff764a60f 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,8 @@ Spec-Driven Development **flips the script** on traditional software development Choose your preferred installation method: +> **Important:** The only official, maintained packages for Spec Kit are published from this GitHub repository. Any packages with the same name on PyPI are **not** affiliated with this project and are not maintained by the Spec Kit maintainers. Always install directly from GitHub as shown below. + #### Option 1: Persistent Installation (Recommended) Install once and use everywhere. Pin a specific release tag for stability (check [Releases](https://github.com/github/spec-kit/releases) for the latest): @@ -62,7 +64,13 @@ uv tool install specify-cli --from git+https://github.com/github/spec-kit.git@vX uv tool install specify-cli --from git+https://github.com/github/spec-kit.git ``` -Then use the tool directly: +Then verify the correct version is installed: + +```bash +specify version +``` + +And use the tool directly: ```bash # Create new project diff --git a/docs/installation.md b/docs/installation.md index 5d560b6e33..25a1074c80 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -10,6 +10,8 @@ ## Installation +> **Important:** The only official, maintained packages for Spec Kit are published from the [github/spec-kit](https://github.com/github/spec-kit) GitHub repository. Any packages with the same name available on PyPI (e.g. `specify-cli` on pypi.org) are **not** affiliated with this project and are not maintained by the Spec Kit maintainers. Always install directly from GitHub as shown below. + ### Initialize a New Project The easiest way to get started is to initialize a new project. Pin a specific release tag for stability (check [Releases](https://github.com/github/spec-kit/releases) for the latest): @@ -69,6 +71,14 @@ uvx --from git+https://github.com/github/spec-kit.git@vX.Y.Z specify init Date: Mon, 30 Mar 2026 22:25:08 +0200 Subject: [PATCH 02/16] fix(extensions): auto-correct legacy command names instead of hard-failing (#2017) Community extensions that predate the strict naming requirement use two common legacy formats ('speckit.command' and 'extension.command'). Instead of rejecting them outright, auto-correct to the required 'speckit.{extension}.{command}' pattern and emit a compatibility warning so authors know they need to update their manifest. Names that cannot be safely corrected (e.g. single-segment names) still raise ValidationError. Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/__init__.py | 4 +++ src/specify_cli/extensions.py | 40 +++++++++++++++++++++++++---- tests/test_extensions.py | 48 ++++++++++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 455beea38e..fd2d80b21d 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -3067,6 +3067,10 @@ def extension_add( console.print("\n[green]✓[/green] Extension installed successfully!") console.print(f"\n[bold]{manifest.name}[/bold] (v{manifest.version})") console.print(f" {manifest.description}") + + for warning in manifest.warnings: + console.print(f"\n[yellow]⚠ Compatibility warning:[/yellow] {warning}") + console.print("\n[bold cyan]Provided commands:[/bold cyan]") for cmd in manifest.commands: console.print(f" • {cmd['name']} - {cmd.get('description', '')}") diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 3420a7651b..76fbee829f 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -130,6 +130,7 @@ def __init__(self, manifest_path: Path): ValidationError: If manifest is invalid """ self.path = manifest_path + self.warnings: List[str] = [] self.data = self._load_yaml(manifest_path) self._validate() @@ -192,11 +193,40 @@ def _validate(self): raise ValidationError("Command missing 'name' or 'file'") # Validate command name format - if EXTENSION_COMMAND_NAME_PATTERN.match(cmd["name"]) is None: - raise ValidationError( - f"Invalid command name '{cmd['name']}': " - "must follow pattern 'speckit.{extension}.{command}'" - ) + if not EXTENSION_COMMAND_NAME_PATTERN.match(cmd["name"]): + corrected = self._try_correct_command_name(cmd["name"], ext["id"]) + if corrected: + self.warnings.append( + f"Command name '{cmd['name']}' does not follow the required pattern " + f"'speckit.{{extension}}.{{command}}'. Registering as '{corrected}'. " + f"The extension author should update the manifest to use this name." + ) + cmd["name"] = corrected + else: + raise ValidationError( + f"Invalid command name '{cmd['name']}': " + "must follow pattern 'speckit.{extension}.{command}'" + ) + + @staticmethod + def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]: + """Try to auto-correct a non-conforming command name to the required pattern. + + Handles the two most common legacy formats used by community extensions: + - 'speckit.command' → 'speckit.{ext_id}.command' + - 'extension.command' → 'speckit.extension.command' + + Returns the corrected name, or None if no safe correction is possible. + """ + parts = name.split('.') + if len(parts) == 2: + if parts[0] == 'speckit': + candidate = f"speckit.{ext_id}.{parts[1]}" + else: + candidate = f"speckit.{parts[0]}.{parts[1]}" + if EXTENSION_COMMAND_NAME_PATTERN.match(candidate): + return candidate + return None @property def id(self) -> str: diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 350b368eac..3714e085c6 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -242,7 +242,7 @@ def test_invalid_version(self, temp_dir, valid_manifest_data): ExtensionManifest(manifest_path) def test_invalid_command_name(self, temp_dir, valid_manifest_data): - """Test manifest with invalid command name format.""" + """Test manifest with command name that cannot be auto-corrected raises ValidationError.""" import yaml valid_manifest_data["provides"]["commands"][0]["name"] = "invalid-name" @@ -254,6 +254,52 @@ def test_invalid_command_name(self, temp_dir, valid_manifest_data): with pytest.raises(ValidationError, match="Invalid command name"): ExtensionManifest(manifest_path) + def test_command_name_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_data): + """Test that 'speckit.command' is auto-corrected to 'speckit.{ext_id}.command'.""" + import yaml + + valid_manifest_data["provides"]["commands"][0]["name"] = "speckit.hello" + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_manifest_data, f) + + manifest = ExtensionManifest(manifest_path) + + assert manifest.commands[0]["name"] == "speckit.test-ext.hello" + assert len(manifest.warnings) == 1 + assert "speckit.hello" in manifest.warnings[0] + assert "speckit.test-ext.hello" in manifest.warnings[0] + + def test_command_name_autocorrect_no_speckit_prefix(self, temp_dir, valid_manifest_data): + """Test that 'extension.command' is auto-corrected to 'speckit.extension.command'.""" + import yaml + + valid_manifest_data["provides"]["commands"][0]["name"] = "docguard.guard" + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_manifest_data, f) + + manifest = ExtensionManifest(manifest_path) + + assert manifest.commands[0]["name"] == "speckit.docguard.guard" + assert len(manifest.warnings) == 1 + assert "docguard.guard" in manifest.warnings[0] + assert "speckit.docguard.guard" in manifest.warnings[0] + + def test_valid_command_name_has_no_warnings(self, temp_dir, valid_manifest_data): + """Test that a correctly-named command produces no warnings.""" + import yaml + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_manifest_data, f) + + manifest = ExtensionManifest(manifest_path) + + assert manifest.warnings == [] + def test_no_commands(self, temp_dir, valid_manifest_data): """Test manifest with no commands provided.""" import yaml From f5aaa7efcc4bad08da273d056419608030805cb9 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Mon, 30 Mar 2026 22:41:19 +0200 Subject: [PATCH 03/16] fix(tests): isolate preset catalog search test from community catalog network calls test_search_with_cached_data asserted exactly 2 results but was getting 4 because _get_merged_packs() queries the full built-in catalog stack (default + community). The community catalog had no local cache and hit the network, returning real presets. Writing a project-level preset-catalogs.yml that pins the test to the default URL only makes the count assertions deterministic. Co-Authored-By: Claude Sonnet 4.6 --- tests/test_presets.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_presets.py b/tests/test_presets.py index d22264f806..1cb8a74cf6 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1180,6 +1180,16 @@ def test_search_with_cached_data(self, project_dir, monkeypatch): catalog = PresetCatalog(project_dir) catalog.cache_dir.mkdir(parents=True, exist_ok=True) + # Restrict to default catalog only — prevents community catalog network calls + # which would add extra results and make the count assertions flaky. + (project_dir / ".specify" / "preset-catalogs.yml").write_text( + f"catalogs:\n" + f" - url: \"{PresetCatalog.DEFAULT_CATALOG_URL}\"\n" + f" name: default\n" + f" priority: 1\n" + f" install_allowed: true\n" + ) + catalog_data = { "schema_version": "1.0", "presets": { From 836388ff6fae2ab337070afe731c44f0e4a8769c Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Mon, 30 Mar 2026 23:47:21 +0200 Subject: [PATCH 04/16] fix(extensions): extend auto-correction to aliases (#2017) The upstream #1994 added alias validation in _collect_manifest_command_names, which also rejected legacy 2-part alias names (e.g. 'speckit.verify'). Extend the same auto-correction logic from _validate() to cover aliases, so both 'speckit.command' and 'extension.command' alias formats are corrected to 'speckit.{ext_id}.{command}' with a compatibility warning instead of hard-failing. Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/extensions.py | 18 ++++++++++++++++++ tests/test_extensions.py | 29 +++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 76fbee829f..cfca34bac4 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -208,6 +208,24 @@ def _validate(self): "must follow pattern 'speckit.{extension}.{command}'" ) + # Validate and auto-correct alias name formats + aliases = cmd.get("aliases") or [] + for i, alias in enumerate(aliases): + if not EXTENSION_COMMAND_NAME_PATTERN.match(alias): + corrected = self._try_correct_command_name(alias, ext["id"]) + if corrected: + self.warnings.append( + f"Alias '{alias}' does not follow the required pattern " + f"'speckit.{{extension}}.{{command}}'. Registering as '{corrected}'. " + f"The extension author should update the manifest to use this name." + ) + aliases[i] = corrected + else: + raise ValidationError( + f"Invalid alias '{alias}': " + "must follow pattern 'speckit.{extension}.{command}'" + ) + @staticmethod def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]: """Try to auto-correct a non-conforming command name to the required pattern. diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 3714e085c6..7aa4dc3976 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -288,6 +288,23 @@ def test_command_name_autocorrect_no_speckit_prefix(self, temp_dir, valid_manife assert "docguard.guard" in manifest.warnings[0] assert "speckit.docguard.guard" in manifest.warnings[0] + def test_alias_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_data): + """Test that a legacy 'speckit.command' alias is auto-corrected.""" + import yaml + + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.hello"] + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_manifest_data, f) + + manifest = ExtensionManifest(manifest_path) + + assert manifest.commands[0]["aliases"] == ["speckit.test-ext.hello"] + assert len(manifest.warnings) == 1 + assert "speckit.hello" in manifest.warnings[0] + assert "speckit.test-ext.hello" in manifest.warnings[0] + def test_valid_command_name_has_no_warnings(self, temp_dir, valid_manifest_data): """Test that a correctly-named command produces no warnings.""" import yaml @@ -682,8 +699,8 @@ def test_install_rejects_extension_id_in_core_namespace(self, temp_dir, project_ with pytest.raises(ValidationError, match="conflicts with core command namespace"): manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) - def test_install_rejects_alias_without_extension_namespace(self, temp_dir, project_dir): - """Install should reject legacy short aliases that can shadow core commands.""" + def test_install_autocorrects_alias_without_extension_namespace(self, temp_dir, project_dir): + """Legacy short aliases are auto-corrected to 'speckit.{ext_id}.{cmd}' with a warning.""" import yaml ext_dir = temp_dir / "alias-shortcut" @@ -714,8 +731,12 @@ def test_install_rejects_alias_without_extension_namespace(self, temp_dir, proje (ext_dir / "commands" / "cmd.md").write_text("---\ndescription: Test\n---\n\nBody") manager = ExtensionManager(project_dir) - with pytest.raises(ValidationError, match="Invalid alias 'speckit.shortcut'"): - manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + manifest = manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + assert manifest.commands[0]["aliases"] == ["speckit.alias-shortcut.shortcut"] + assert len(manifest.warnings) == 1 + assert "speckit.shortcut" in manifest.warnings[0] + assert "speckit.alias-shortcut.shortcut" in manifest.warnings[0] def test_install_rejects_namespace_squatting(self, temp_dir, project_dir): """Install should reject commands and aliases outside the extension namespace.""" From 01bb599eb651018a617e97fa1bb866b272b3d2c9 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Tue, 31 Mar 2026 21:31:16 +0200 Subject: [PATCH 05/16] fix(extensions): address PR review feedback (#2017) - _try_correct_command_name: only correct 'X.Y' to 'speckit.ext_id.Y' when X matches ext_id, preventing misleading warnings followed by install failure due to namespace mismatch - _validate: add aliases type/string guards matching _collect_manifest _command_names defensive checks - _validate: track command renames and rewrite any hook.*.command references that pointed at a renamed command, emitting a warning - test: fix test_command_name_autocorrect_no_speckit_prefix to use ext_id matching the legacy namespace; add namespace-mismatch test - test: replace redundant preset-catalogs.yml isolation with monkeypatch.delenv("SPECKIT_PRESET_CATALOG_URL") so the env var cannot bypass catalog restriction in CI environments Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/extensions.py | 46 +++++++++++++++++++++++++++-------- tests/test_extensions.py | 20 +++++++++++++-- tests/test_presets.py | 13 +--------- 3 files changed, 55 insertions(+), 24 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index cfca34bac4..ab4f009dbb 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -187,7 +187,8 @@ def _validate(self): if "commands" not in provides or not provides["commands"]: raise ValidationError("Extension must provide at least one command") - # Validate commands + # Validate commands; track renames so hook references can be rewritten. + rename_map: Dict[str, str] = {} for cmd in provides["commands"]: if "name" not in cmd or "file" not in cmd: raise ValidationError("Command missing 'name' or 'file'") @@ -201,6 +202,7 @@ def _validate(self): f"'speckit.{{extension}}.{{command}}'. Registering as '{corrected}'. " f"The extension author should update the manifest to use this name." ) + rename_map[cmd["name"]] = corrected cmd["name"] = corrected else: raise ValidationError( @@ -209,8 +211,18 @@ def _validate(self): ) # Validate and auto-correct alias name formats - aliases = cmd.get("aliases") or [] + aliases = cmd.get("aliases") + if aliases is None: + aliases = [] + if not isinstance(aliases, list): + raise ValidationError( + f"Aliases for command '{cmd['name']}' must be a list" + ) for i, alias in enumerate(aliases): + if not isinstance(alias, str): + raise ValidationError( + f"Aliases for command '{cmd['name']}' must be strings" + ) if not EXTENSION_COMMAND_NAME_PATTERN.match(alias): corrected = self._try_correct_command_name(alias, ext["id"]) if corrected: @@ -219,6 +231,7 @@ def _validate(self): f"'speckit.{{extension}}.{{command}}'. Registering as '{corrected}'. " f"The extension author should update the manifest to use this name." ) + rename_map[alias] = corrected aliases[i] = corrected else: raise ValidationError( @@ -226,24 +239,37 @@ def _validate(self): "must follow pattern 'speckit.{extension}.{command}'" ) + # Rewrite any hook command references that pointed at a renamed command. + for hook_name, hook_data in self.data.get("hooks", {}).items(): + if isinstance(hook_data, dict) and hook_data.get("command") in rename_map: + old_ref = hook_data["command"] + hook_data["command"] = rename_map[old_ref] + self.warnings.append( + f"Hook '{hook_name}' referenced renamed command '{old_ref}'; " + f"updated to '{rename_map[old_ref]}'. " + f"The extension author should update the manifest." + ) + @staticmethod def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]: """Try to auto-correct a non-conforming command name to the required pattern. - Handles the two most common legacy formats used by community extensions: - - 'speckit.command' → 'speckit.{ext_id}.command' - - 'extension.command' → 'speckit.extension.command' + Handles the two legacy formats used by community extensions: + - 'speckit.command' → 'speckit.{ext_id}.command' + - '{ext_id}.command' → 'speckit.{ext_id}.command' + + The 'X.Y' form is only corrected when X matches ext_id to ensure the + result passes the install-time namespace check. Any other prefix is + uncorrectable and will produce a ValidationError at the call site. Returns the corrected name, or None if no safe correction is possible. """ parts = name.split('.') if len(parts) == 2: - if parts[0] == 'speckit': + if parts[0] == 'speckit' or parts[0] == ext_id: candidate = f"speckit.{ext_id}.{parts[1]}" - else: - candidate = f"speckit.{parts[0]}.{parts[1]}" - if EXTENSION_COMMAND_NAME_PATTERN.match(candidate): - return candidate + if EXTENSION_COMMAND_NAME_PATTERN.match(candidate): + return candidate return None @property diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 7aa4dc3976..a14750c656 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -271,10 +271,12 @@ def test_command_name_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_ assert "speckit.hello" in manifest.warnings[0] assert "speckit.test-ext.hello" in manifest.warnings[0] - def test_command_name_autocorrect_no_speckit_prefix(self, temp_dir, valid_manifest_data): - """Test that 'extension.command' is auto-corrected to 'speckit.extension.command'.""" + def test_command_name_autocorrect_matching_ext_id_prefix(self, temp_dir, valid_manifest_data): + """Test that '{ext_id}.command' is auto-corrected to 'speckit.{ext_id}.command'.""" import yaml + # Set ext_id to match the legacy namespace so correction is valid + valid_manifest_data["extension"]["id"] = "docguard" valid_manifest_data["provides"]["commands"][0]["name"] = "docguard.guard" manifest_path = temp_dir / "extension.yml" @@ -288,6 +290,20 @@ def test_command_name_autocorrect_no_speckit_prefix(self, temp_dir, valid_manife assert "docguard.guard" in manifest.warnings[0] assert "speckit.docguard.guard" in manifest.warnings[0] + def test_command_name_mismatched_namespace_not_corrected(self, temp_dir, valid_manifest_data): + """Test that 'X.command' is NOT corrected when X doesn't match ext_id.""" + import yaml + + # ext_id is "test-ext" but command uses a different namespace + valid_manifest_data["provides"]["commands"][0]["name"] = "docguard.guard" + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_manifest_data, f) + + with pytest.raises(ValidationError, match="Invalid command name"): + ExtensionManifest(manifest_path) + def test_alias_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_data): """Test that a legacy 'speckit.command' alias is auto-corrected.""" import yaml diff --git a/tests/test_presets.py b/tests/test_presets.py index 1cb8a74cf6..57e209f407 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1175,21 +1175,10 @@ def test_search_with_cached_data(self, project_dir, monkeypatch): """Test search with cached catalog data.""" from unittest.mock import patch - # Only use the default catalog to prevent fetching the community catalog from the network - monkeypatch.setenv("SPECKIT_PRESET_CATALOG_URL", PresetCatalog.DEFAULT_CATALOG_URL) + monkeypatch.delenv("SPECKIT_PRESET_CATALOG_URL", raising=False) catalog = PresetCatalog(project_dir) catalog.cache_dir.mkdir(parents=True, exist_ok=True) - # Restrict to default catalog only — prevents community catalog network calls - # which would add extra results and make the count assertions flaky. - (project_dir / ".specify" / "preset-catalogs.yml").write_text( - f"catalogs:\n" - f" - url: \"{PresetCatalog.DEFAULT_CATALOG_URL}\"\n" - f" name: default\n" - f" priority: 1\n" - f" install_allowed: true\n" - ) - catalog_data = { "schema_version": "1.0", "presets": { From caee3e02cae82f39b7e9d1a537157f17806cf07e Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 08:04:09 +0200 Subject: [PATCH 06/16] 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 --- src/specify_cli/extensions.py | 68 ++++++++++++++++++++++++++-------- tests/test_extensions.py | 70 ++++++++++++++++++++++++++++------- 2 files changed, 109 insertions(+), 29 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index ab4f009dbb..5b15d6ecb3 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -37,6 +37,9 @@ "taskstoissues", }) EXTENSION_COMMAND_NAME_PATTERN = re.compile(r"^speckit\.([a-z0-9-]+)\.([a-z0-9-]+)$") +# Aliases use a 2-part 'namespace.command' form without the 'speckit.' prefix. +# The 'speckit' namespace is reserved for core commands. +EXTENSION_ALIAS_PATTERN = re.compile(r"^([a-z0-9-]+)\.([a-z0-9-]+)$") def _load_core_command_names() -> frozenset[str]: @@ -214,6 +217,7 @@ def _validate(self): aliases = cmd.get("aliases") if aliases is None: aliases = [] + cmd["aliases"] = [] # normalize null/missing to empty list in-place if not isinstance(aliases, list): raise ValidationError( f"Aliases for command '{cmd['name']}' must be a list" @@ -223,12 +227,15 @@ def _validate(self): raise ValidationError( f"Aliases for command '{cmd['name']}' must be strings" ) - if not EXTENSION_COMMAND_NAME_PATTERN.match(alias): - corrected = self._try_correct_command_name(alias, ext["id"]) + alias_match = EXTENSION_ALIAS_PATTERN.match(alias) + if alias_match and alias_match.group(1) != 'speckit': + pass # already valid: 'myext.command' form + else: + corrected = self._try_correct_alias_name(alias, ext["id"]) if corrected: self.warnings.append( f"Alias '{alias}' does not follow the required pattern " - f"'speckit.{{extension}}.{{command}}'. Registering as '{corrected}'. " + f"'{{extension}}.{{command}}'. Registering as '{corrected}'. " f"The extension author should update the manifest to use this name." ) rename_map[alias] = corrected @@ -236,7 +243,8 @@ def _validate(self): else: raise ValidationError( f"Invalid alias '{alias}': " - "must follow pattern 'speckit.{extension}.{command}'" + "must follow pattern '{extension}.{command}' " + "(the 'speckit.' prefix is reserved for core commands)" ) # Rewrite any hook command references that pointed at a renamed command. @@ -252,15 +260,14 @@ def _validate(self): @staticmethod def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]: - """Try to auto-correct a non-conforming command name to the required pattern. + """Try to auto-correct a non-conforming primary command name to 'speckit.{ext_id}.command'. - Handles the two legacy formats used by community extensions: + Handles legacy formats: - 'speckit.command' → 'speckit.{ext_id}.command' - '{ext_id}.command' → 'speckit.{ext_id}.command' The 'X.Y' form is only corrected when X matches ext_id to ensure the - result passes the install-time namespace check. Any other prefix is - uncorrectable and will produce a ValidationError at the call site. + result passes the install-time namespace check. Returns the corrected name, or None if no safe correction is possible. """ @@ -272,6 +279,27 @@ def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]: return candidate return None + @staticmethod + def _try_correct_alias_name(name: str, ext_id: str) -> Optional[str]: + """Try to auto-correct a non-conforming alias to the '{ext_id}.command' pattern. + + Handles legacy formats: + - 'speckit.command' → '{ext_id}.command' + - 'speckit.ext_id.command' → 'ext_id.command' (3-part with speckit prefix) + + Returns the corrected name, or None if no safe correction is possible. + """ + parts = name.split('.') + if len(parts) == 2 and parts[0] == 'speckit': + candidate = f"{ext_id}.{parts[1]}" + if EXTENSION_ALIAS_PATTERN.match(candidate): + return candidate + if len(parts) == 3 and parts[0] == 'speckit': + candidate = f"{parts[1]}.{parts[2]}" + if EXTENSION_ALIAS_PATTERN.match(candidate): + return candidate + return None + @property def id(self) -> str: """Get extension ID.""" @@ -608,14 +636,24 @@ def _collect_manifest_command_names(manifest: ExtensionManifest) -> Dict[str, st f"{kind.capitalize()} for command '{primary_name}' must be a string" ) - match = EXTENSION_COMMAND_NAME_PATTERN.match(name) - if match is None: - raise ValidationError( - f"Invalid {kind} '{name}': " - "must follow pattern 'speckit.{extension}.{command}'" - ) + if kind == "command": + match = EXTENSION_COMMAND_NAME_PATTERN.match(name) + if match is None: + raise ValidationError( + f"Invalid command '{name}': " + "must follow pattern 'speckit.{extension}.{command}'" + ) + namespace = match.group(1) + else: + match = EXTENSION_ALIAS_PATTERN.match(name) + if match is None or match.group(1) == 'speckit': + raise ValidationError( + f"Invalid alias '{name}': " + "must follow pattern '{extension}.{command}' " + "(the 'speckit.' prefix is reserved for core commands)" + ) + namespace = match.group(1) - namespace = match.group(1) if namespace != manifest.id: raise ValidationError( f"{kind.capitalize()} '{name}' must use extension namespace '{manifest.id}'" diff --git a/tests/test_extensions.py b/tests/test_extensions.py index a14750c656..b5d3dbb61b 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -304,8 +304,23 @@ def test_command_name_mismatched_namespace_not_corrected(self, temp_dir, valid_m with pytest.raises(ValidationError, match="Invalid command name"): ExtensionManifest(manifest_path) - def test_alias_autocorrect_speckit_prefix(self, temp_dir, valid_manifest_data): - """Test that a legacy 'speckit.command' alias is auto-corrected.""" + def test_alias_valid_two_part_no_prefix(self, temp_dir, valid_manifest_data): + """Test that a 'myext.command' alias is accepted as-is with no warning.""" + import yaml + + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.hello"] + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_manifest_data, f) + + manifest = ExtensionManifest(manifest_path) + + assert manifest.commands[0]["aliases"] == ["test-ext.hello"] + assert manifest.warnings == [] + + def test_alias_autocorrect_speckit_two_part(self, temp_dir, valid_manifest_data): + """Test that legacy 'speckit.command' alias is corrected to '{ext_id}.command'.""" import yaml 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): manifest = ExtensionManifest(manifest_path) - assert manifest.commands[0]["aliases"] == ["speckit.test-ext.hello"] + assert manifest.commands[0]["aliases"] == ["test-ext.hello"] assert len(manifest.warnings) == 1 assert "speckit.hello" in manifest.warnings[0] + assert "test-ext.hello" in manifest.warnings[0] + + def test_alias_autocorrect_speckit_three_part(self, temp_dir, valid_manifest_data): + """Test that a 3-part 'speckit.ext.command' alias is corrected to 'ext.command'.""" + import yaml + + # Clear hooks so the alias rename doesn't also trigger a hook-reference warning. + valid_manifest_data.pop("hooks", None) + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.test-ext.hello"] + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, 'w') as f: + yaml.dump(valid_manifest_data, f) + + manifest = ExtensionManifest(manifest_path) + + assert manifest.commands[0]["aliases"] == ["test-ext.hello"] + assert len(manifest.warnings) == 1 assert "speckit.test-ext.hello" in manifest.warnings[0] + assert "test-ext.hello" in manifest.warnings[0] def test_valid_command_name_has_no_warnings(self, temp_dir, valid_manifest_data): """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_ with pytest.raises(ValidationError, match="conflicts with core command namespace"): manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) - def test_install_autocorrects_alias_without_extension_namespace(self, temp_dir, project_dir): - """Legacy short aliases are auto-corrected to 'speckit.{ext_id}.{cmd}' with a warning.""" + def test_install_autocorrects_speckit_alias_to_ext_form(self, temp_dir, project_dir): + """Legacy 'speckit.command' aliases are corrected to '{ext_id}.command' with a warning.""" import yaml ext_dir = temp_dir / "alias-shortcut" @@ -749,10 +783,10 @@ def test_install_autocorrects_alias_without_extension_namespace(self, temp_dir, manager = ExtensionManager(project_dir) manifest = manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) - assert manifest.commands[0]["aliases"] == ["speckit.alias-shortcut.shortcut"] + assert manifest.commands[0]["aliases"] == ["alias-shortcut.shortcut"] assert len(manifest.warnings) == 1 assert "speckit.shortcut" in manifest.warnings[0] - assert "speckit.alias-shortcut.shortcut" in manifest.warnings[0] + assert "alias-shortcut.shortcut" in manifest.warnings[0] def test_install_rejects_namespace_squatting(self, temp_dir, project_dir): """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): manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) def test_install_rejects_command_collision_with_installed_extension(self, temp_dir, project_dir): - """Install should reject names already claimed by an installed legacy extension.""" + """Install should reject names already claimed by an installed extension. + + The first extension is written directly to disk (bypassing install_from_directory) to + simulate a scenario where it claims a command in a different namespace — something that + could happen with a legacy or manually-placed extension. The collision detector must + still catch the conflict when the second extension tries to register the same command. + """ import yaml first_dir = temp_dir / "ext-one" first_dir.mkdir() (first_dir / "commands").mkdir() + # Directly write a manifest that claims speckit.shared.sync even though its id is + # "ext-one" — this bypasses install-time namespace enforcement and simulates a + # legacy/corrupted extension that already occupies the name. first_manifest = { "schema_version": "1.0", "extension": { @@ -808,9 +851,8 @@ def test_install_rejects_command_collision_with_installed_extension(self, temp_d "provides": { "commands": [ { - "name": "speckit.ext-one.sync", + "name": "speckit.shared.sync", "file": "commands/cmd.md", - "aliases": ["speckit.shared.sync"], } ] }, @@ -1148,7 +1190,7 @@ def test_command_with_aliases(self, project_dir, temp_dir): { "name": "speckit.ext-alias.cmd", "file": "commands/cmd.md", - "aliases": ["speckit.ext-alias.shortcut"], + "aliases": ["ext-alias.shortcut"], } ] }, @@ -1169,7 +1211,7 @@ def test_command_with_aliases(self, project_dir, temp_dir): assert len(registered) == 2 assert "speckit.ext-alias.cmd" in registered - assert "speckit.ext-alias.shortcut" in registered + assert "ext-alias.shortcut" in registered assert (claude_dir / "speckit-ext-alias-cmd" / "SKILL.md").exists() assert (claude_dir / "speckit-ext-alias-shortcut" / "SKILL.md").exists() @@ -1591,7 +1633,7 @@ def test_copilot_aliases_get_companion_prompts(self, project_dir, temp_dir): { "name": "speckit.ext-alias-copilot.cmd", "file": "commands/cmd.md", - "aliases": ["speckit.ext-alias-copilot.shortcut"], + "aliases": ["ext-alias-copilot.shortcut"], } ] }, @@ -1619,7 +1661,7 @@ def test_copilot_aliases_get_companion_prompts(self, project_dir, temp_dir): # Both primary and alias get companion .prompt.md prompts_dir = project_dir / ".github" / "prompts" assert (prompts_dir / "speckit.ext-alias-copilot.cmd.prompt.md").exists() - assert (prompts_dir / "speckit.ext-alias-copilot.shortcut.prompt.md").exists() + assert (prompts_dir / "ext-alias-copilot.shortcut.prompt.md").exists() def test_non_copilot_agent_no_companion_file(self, extension_dir, project_dir): """Test that non-copilot agents do NOT create .prompt.md files.""" From 240f715194ef9f3338b0f3fbe12d2446993140ba Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 12:14:41 +0200 Subject: [PATCH 07/16] fix(extensions): validate hooks is a dict and restrict alias 3-part correction to ext_id (#2017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Raise ValidationError with a clear message if the manifest 'hooks' value is not a mapping (previously would AttributeError on .items()) - Restrict _try_correct_alias_name 3-part correction (speckit.X.Y → X.Y) to only apply when X == ext_id; when X differs the alias is squatting a foreign namespace and should be rejected, not silently rewritten Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/extensions.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 5b15d6ecb3..9c710ce33f 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -248,7 +248,12 @@ def _validate(self): ) # Rewrite any hook command references that pointed at a renamed command. - for hook_name, hook_data in self.data.get("hooks", {}).items(): + hooks = self.data.get("hooks", {}) + if not isinstance(hooks, dict): + raise ValidationError( + f"'hooks' must be a mapping, got {type(hooks).__name__}" + ) + for hook_name, hook_data in hooks.items(): if isinstance(hook_data, dict) and hook_data.get("command") in rename_map: old_ref = hook_data["command"] hook_data["command"] = rename_map[old_ref] @@ -294,7 +299,7 @@ def _try_correct_alias_name(name: str, ext_id: str) -> Optional[str]: candidate = f"{ext_id}.{parts[1]}" if EXTENSION_ALIAS_PATTERN.match(candidate): return candidate - if len(parts) == 3 and parts[0] == 'speckit': + if len(parts) == 3 and parts[0] == 'speckit' and parts[1] == ext_id: candidate = f"{parts[1]}.{parts[2]}" if EXTENSION_ALIAS_PATTERN.match(candidate): return candidate From 5a152ea620aae8d4e744d92cb7bebd0c32247d89 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 16:47:49 +0200 Subject: [PATCH 08/16] 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 --- src/specify_cli/extensions.py | 33 ++++++++++++++++++++++++++------- tests/test_extensions.py | 32 +++++++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 9c710ce33f..bace629ecc 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -247,21 +247,39 @@ def _validate(self): "(the 'speckit.' prefix is reserved for core commands)" ) - # Rewrite any hook command references that pointed at a renamed command. + # Rewrite any hook command references that pointed at a renamed command, + # then canonicalize alias-form refs (e.g. 'ext.cmd') to the canonical + # 'speckit.{ext}.cmd' form so hook-invocation skill-name mapping works. hooks = self.data.get("hooks", {}) if not isinstance(hooks, dict): raise ValidationError( f"'hooks' must be a mapping, got {type(hooks).__name__}" ) for hook_name, hook_data in hooks.items(): - if isinstance(hook_data, dict) and hook_data.get("command") in rename_map: - old_ref = hook_data["command"] - hook_data["command"] = rename_map[old_ref] + if not isinstance(hook_data, dict): + continue + command_ref = hook_data.get("command") + if not isinstance(command_ref, str): + continue + + # Step 1: apply rename_map (command was auto-corrected during validation). + rewritten = rename_map.get(command_ref, command_ref) + if rewritten != command_ref: + hook_data["command"] = rewritten self.warnings.append( - f"Hook '{hook_name}' referenced renamed command '{old_ref}'; " - f"updated to '{rename_map[old_ref]}'. " + f"Hook '{hook_name}' referenced renamed command '{command_ref}'; " + f"updated to '{rewritten}'. " f"The extension author should update the manifest." ) + command_ref = rewritten + + # Step 2: if the ref is in alias form '{ext_id}.cmd', lift it to + # 'speckit.{ext_id}.cmd' so skill-mode hook invocation renders correctly. + parts = command_ref.split('.') + if len(parts) == 2 and parts[0] == ext["id"]: + canonical = f"speckit.{ext['id']}.{parts[1]}" + if EXTENSION_COMMAND_NAME_PATTERN.match(canonical): + hook_data["command"] = canonical @staticmethod 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 """Collect command and alias names declared by a manifest. Performs install-time validation for extension-specific constraints: - - commands and aliases must use the canonical `speckit.{extension}.{command}` shape + - primary commands must use the canonical `speckit.{extension}.{command}` shape + - aliases must use the `{extension}.{command}` shape (no `speckit.` prefix) - commands and aliases must use this extension's namespace - command namespaces must not shadow core commands - duplicate command/alias names inside one manifest are rejected diff --git a/tests/test_extensions.py b/tests/test_extensions.py index b5d3dbb61b..7946fbe3f4 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -305,7 +305,7 @@ def test_command_name_mismatched_namespace_not_corrected(self, temp_dir, valid_m ExtensionManifest(manifest_path) def test_alias_valid_two_part_no_prefix(self, temp_dir, valid_manifest_data): - """Test that a 'myext.command' alias is accepted as-is with no warning.""" + """Test that a 'test-ext.hello' alias is accepted as-is with no warning.""" import yaml 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 assert "speckit.test-ext.hello" in manifest.warnings[0] assert "test-ext.hello" in manifest.warnings[0] + def test_hook_alias_ref_canonicalized_to_speckit_form(self, temp_dir, valid_manifest_data): + """Hook command refs in alias form are lifted to canonical speckit.ext.cmd form.""" + import yaml + + # Manifest uses a valid alias but the hook mistakenly references the alias name. + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.hello"] + valid_manifest_data["hooks"]["after_tasks"]["command"] = "test-ext.hello" + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, "w") as f: + yaml.dump(valid_manifest_data, f) + + manifest = ExtensionManifest(manifest_path) + + # Hook ref should be lifted to canonical form for skill-mode invocation. + assert manifest.hooks["after_tasks"]["command"] == "speckit.test-ext.hello" + + def test_hook_non_dict_value_raises(self, temp_dir, valid_manifest_data): + """A non-mapping hooks value raises ValidationError.""" + import yaml + + valid_manifest_data["hooks"] = ["not", "a", "dict"] + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, "w") as f: + yaml.dump(valid_manifest_data, f) + + with pytest.raises(ValidationError, match="'hooks' must be a mapping"): + ExtensionManifest(manifest_path) + def test_valid_command_name_has_no_warnings(self, temp_dir, valid_manifest_data): """Test that a correctly-named command produces no warnings.""" import yaml From c870cb82c7437634f8ee91a02105b24d70ec8300 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 17:29:02 +0200 Subject: [PATCH 09/16] fix(agents): remove empty skill parent dir on unregister (#2017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When unregistering a SKILL.md-based command (claude, codex, kimi, agy), the SKILL.md file was unlinked but the now-empty parent directory (e.g. .claude/skills/speckit-myext-run/) was left behind. After unlinking the file, attempt rmdir on the parent — a best-effort cleanup that is silently ignored if the directory is non-empty or already gone. Add test_unregister_skill_removes_parent_directory to cover this. Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/agents.py | 8 ++++++++ tests/test_extensions.py | 15 +++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index bb25b3fc1a..26ba70cb57 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -553,6 +553,14 @@ def unregister_commands( cmd_file = commands_dir / f"{output_name}{agent_config['extension']}" if cmd_file.exists(): cmd_file.unlink() + # For SKILL.md agents the file lives in a subdirectory + # (e.g. .claude/skills/speckit-foo/SKILL.md). Remove the + # now-empty parent directory so no orphaned dirs remain. + if agent_config["extension"] == "/SKILL.md": + try: + cmd_file.parent.rmdir() + except OSError: + pass # not empty or already gone — leave it if agent_name == "copilot": prompt_file = project_root / ".github" / "prompts" / f"{cmd_name}.prompt.md" diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 7946fbe3f4..17a03b000e 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -1262,6 +1262,21 @@ def test_unregister_commands_for_codex_skills_uses_mapped_names(self, project_di assert not (skills_dir / "speckit-specify" / "SKILL.md").exists() assert not (skills_dir / "speckit-shortcut" / "SKILL.md").exists() + def test_unregister_skill_removes_parent_directory(self, project_dir): + """Removing a SKILL.md-based command also removes the empty parent directory.""" + skills_dir = project_dir / ".agents" / "skills" + (skills_dir / "speckit-myext-run").mkdir(parents=True) + (skills_dir / "speckit-myext-run" / "SKILL.md").write_text("body") + + registrar = CommandRegistrar() + registrar.unregister_commands( + {"codex": ["speckit.myext.run"]}, + project_dir, + ) + + assert not (skills_dir / "speckit-myext-run" / "SKILL.md").exists() + assert not (skills_dir / "speckit-myext-run").exists() + def test_register_commands_for_all_agents_distinguishes_codex_from_amp(self, extension_dir, project_dir): """A Codex project under .agents/skills should not implicitly activate Amp.""" skills_dir = project_dir / ".agents" / "skills" From e80aaeb27c234dec36415ed94bfe76d8b046ff40 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 17:33:23 +0200 Subject: [PATCH 10/16] fix(cli): count aliases in removal confirmation command count (#2017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "N commands from AI agent" line in the removal confirmation was derived from len(manifest.commands) which counts command entries, not registered names. Extensions with aliases therefore showed the wrong count (e.g. 1 instead of 2 for a command with one alias). Use the registry's registered_commands dict instead — summing actual registered names across all agents — so the count reflects what will actually be removed. Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/__init__.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index fd2d80b21d..f41e818ff7 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -3123,9 +3123,13 @@ def extension_remove( extension_id, display_name = _resolve_installed_extension(extension, installed, "remove") # Get extension info for command and skill counts - ext_manifest = manager.get_extension(extension_id) - cmd_count = len(ext_manifest.commands) if ext_manifest else 0 reg_meta = manager.registry.get(extension_id) + # Count total registered commands across all agents (includes aliases) + registered_commands = reg_meta.get("registered_commands", {}) if reg_meta else {} + cmd_count = sum( + len(names) for names in registered_commands.values() + if isinstance(names, list) + ) raw_skills = reg_meta.get("registered_skills") if reg_meta else None skill_count = len(raw_skills) if isinstance(raw_skills, list) else 0 From b80ce0d3c6b2259f336bf6b313eb8c5632e19f56 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 17:35:46 +0200 Subject: [PATCH 11/16] test(cli): add isolated CLI tests for extension remove confirmation count (#2017) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestExtensionRemoveCLI covers two cases: - No aliases: 1 primary command → "1 commands from AI agent" - With alias: 1 primary + 1 alias → "2 commands from AI agent" (not "1") The second test explicitly guards against the regression where only manifest.commands length was used instead of the registry's registered command names. Co-Authored-By: Claude Sonnet 4.6 --- tests/test_extensions.py | 80 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 17a03b000e..a6c719c381 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3101,6 +3101,86 @@ def mock_download(extension_id): ) +class TestExtensionRemoveCLI: + """CLI integration tests for extension remove confirmation output.""" + + def test_remove_confirmation_counts_primary_command_only(self, extension_dir, project_dir): + """Removal confirmation shows correct count when extension has no aliases.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Install via manager so registry is fully populated + (project_dir / ".claude" / "skills").mkdir(parents=True) + manager = ExtensionManager(project_dir) + manager.install_from_directory(extension_dir, "0.1.0", register_commands=True) + + # Invoke remove with 'n' so we only see the confirmation prompt + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["extension", "remove", "test-ext"], input="n\n") + + plain = strip_ansi(result.output) + # The fixture has 1 command and 0 aliases → "1 commands from AI agent" + assert "1 commands from AI agent" in plain + + def test_remove_confirmation_counts_aliases(self, temp_dir, project_dir): + """Removal confirmation shows primary + alias count, not just primary.""" + import yaml + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + runner = CliRunner() + + # Build an extension with 1 primary command + 1 alias + ext_dir = temp_dir / "ext-with-alias" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "ext-with-alias", + "name": "Extension With Alias", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.ext-with-alias.run", + "file": "commands/run.md", + "aliases": ["ext-with-alias.go"], + } + ] + }, + } + (ext_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + (ext_dir / "commands" / "run.md").write_text("---\ndescription: Run\n---\nBody") + + # Install with command registration so registry captures both names + (project_dir / ".claude" / "skills").mkdir(parents=True) + manager = ExtensionManager(project_dir) + manager.install_from_directory(ext_dir, "0.1.0", register_commands=True) + + # Verify registry recorded both names + meta = manager.registry.get("ext-with-alias") + claude_cmds = meta["registered_commands"].get("claude", []) + assert "speckit.ext-with-alias.run" in claude_cmds + assert "ext-with-alias.go" in claude_cmds + + # Invoke remove with 'n' so we only see the confirmation prompt + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke(app, ["extension", "remove", "ext-with-alias"], input="n\n") + + plain = strip_ansi(result.output) + # 1 primary + 1 alias = "2 commands from AI agent" (not "1") + assert "2 commands from AI agent" in plain + assert "1 commands from AI agent" not in plain + + class TestExtensionUpdateCLI: """CLI integration tests for extension update command.""" From 9ab59cd70539534de0fbed882eb7ae0bd6edeba9 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 18:17:23 +0200 Subject: [PATCH 12/16] fix(extensions): address final reviewer comments (#2017) - Reject aliases whose SKILL output name would collide with the primary command (e.g. primary 'speckit.myext.run' + alias 'myext.run' both map to skill 'speckit-myext-run') - Fix hook rewrite warning to show the final canonical value instead of the intermediate rename_map value; compute final_ref (rename + lift) first, then write to hook_data and emit warning in a single step - Guard registered_commands against non-dict in extension_remove CLI (corrupted/legacy registry entries no longer crash with AttributeError) - Update alias tests to use non-colliding alias suffix ('greet' not 'hello') - Add test_alias_collision_with_primary_skill_name_rejected Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/__init__.py | 7 +++-- src/specify_cli/extensions.py | 39 ++++++++++++++++++-------- tests/test_extensions.py | 52 +++++++++++++++++++++++++---------- 3 files changed, 69 insertions(+), 29 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index f41e818ff7..49927e0b5d 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -3124,8 +3124,11 @@ def extension_remove( # Get extension info for command and skill counts reg_meta = manager.registry.get(extension_id) - # Count total registered commands across all agents (includes aliases) - registered_commands = reg_meta.get("registered_commands", {}) if reg_meta else {} + # Count total registered commands across all agents (includes aliases). + # Normalize to dict in case registry entry is corrupted or from an older version. + registered_commands = reg_meta.get("registered_commands") if reg_meta else None + if not isinstance(registered_commands, dict): + registered_commands = {} cmd_count = sum( len(names) for names in registered_commands.values() if isinstance(names, list) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index bace629ecc..9c94f788c0 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -229,7 +229,21 @@ def _validate(self): ) alias_match = EXTENSION_ALIAS_PATTERN.match(alias) if alias_match and alias_match.group(1) != 'speckit': - pass # already valid: 'myext.command' form + # Valid 'ext.cmd' form — check it won't collide with the primary + # command's SKILL output name. For SKILL.md agents, + # _compute_output_name strips 'speckit.' so e.g. primary + # 'speckit.myext.run' and alias 'myext.run' both map to + # 'speckit-myext-run', causing the alias write to overwrite + # the primary skill file. + primary = cmd["name"] + if primary.startswith("speckit.") and primary[len("speckit."):] == alias: + skill_name = "speckit-" + alias.replace(".", "-") + raise ValidationError( + f"Alias '{alias}' would collide with primary command " + f"'{primary}' on SKILL.md-based agents (both map to " + f"'{skill_name}'). " + f"Choose a distinct alias name." + ) else: corrected = self._try_correct_alias_name(alias, ext["id"]) if corrected: @@ -263,23 +277,24 @@ def _validate(self): continue # Step 1: apply rename_map (command was auto-corrected during validation). - rewritten = rename_map.get(command_ref, command_ref) - if rewritten != command_ref: - hook_data["command"] = rewritten - self.warnings.append( - f"Hook '{hook_name}' referenced renamed command '{command_ref}'; " - f"updated to '{rewritten}'. " - f"The extension author should update the manifest." - ) - command_ref = rewritten + final_ref = rename_map.get(command_ref, command_ref) # Step 2: if the ref is in alias form '{ext_id}.cmd', lift it to # 'speckit.{ext_id}.cmd' so skill-mode hook invocation renders correctly. - parts = command_ref.split('.') + parts = final_ref.split('.') if len(parts) == 2 and parts[0] == ext["id"]: canonical = f"speckit.{ext['id']}.{parts[1]}" if EXTENSION_COMMAND_NAME_PATTERN.match(canonical): - hook_data["command"] = canonical + final_ref = canonical + + if final_ref != command_ref: + hook_data["command"] = final_ref + if command_ref in rename_map: + self.warnings.append( + f"Hook '{hook_name}' referenced renamed command '{command_ref}'; " + f"updated to '{final_ref}'. " + f"The extension author should update the manifest." + ) @staticmethod def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]: diff --git a/tests/test_extensions.py b/tests/test_extensions.py index a6c719c381..c9ceb731df 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -305,10 +305,12 @@ def test_command_name_mismatched_namespace_not_corrected(self, temp_dir, valid_m ExtensionManifest(manifest_path) def test_alias_valid_two_part_no_prefix(self, temp_dir, valid_manifest_data): - """Test that a 'test-ext.hello' alias is accepted as-is with no warning.""" + """Test that a 'test-ext.greet' alias is accepted as-is with no warning.""" import yaml - valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.hello"] + # Use a name distinct from the primary command's suffix ('hello') to avoid + # the SKILL.md output-name collision (both would map to speckit-test-ext-hello). + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.greet"] manifest_path = temp_dir / "extension.yml" with open(manifest_path, 'w') as f: @@ -316,14 +318,16 @@ def test_alias_valid_two_part_no_prefix(self, temp_dir, valid_manifest_data): manifest = ExtensionManifest(manifest_path) - assert manifest.commands[0]["aliases"] == ["test-ext.hello"] + assert manifest.commands[0]["aliases"] == ["test-ext.greet"] assert manifest.warnings == [] def test_alias_autocorrect_speckit_two_part(self, temp_dir, valid_manifest_data): """Test that legacy 'speckit.command' alias is corrected to '{ext_id}.command'.""" import yaml - valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.hello"] + # Use 'speckit.greet' → 'test-ext.greet' to avoid colliding with the primary + # command's SKILL output name (speckit.test-ext.hello → speckit-test-ext-hello). + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.greet"] manifest_path = temp_dir / "extension.yml" with open(manifest_path, 'w') as f: @@ -331,18 +335,20 @@ def test_alias_autocorrect_speckit_two_part(self, temp_dir, valid_manifest_data) manifest = ExtensionManifest(manifest_path) - assert manifest.commands[0]["aliases"] == ["test-ext.hello"] + assert manifest.commands[0]["aliases"] == ["test-ext.greet"] assert len(manifest.warnings) == 1 - assert "speckit.hello" in manifest.warnings[0] - assert "test-ext.hello" in manifest.warnings[0] + assert "speckit.greet" in manifest.warnings[0] + assert "test-ext.greet" in manifest.warnings[0] def test_alias_autocorrect_speckit_three_part(self, temp_dir, valid_manifest_data): """Test that a 3-part 'speckit.ext.command' alias is corrected to 'ext.command'.""" import yaml # Clear hooks so the alias rename doesn't also trigger a hook-reference warning. + # Use 'speckit.test-ext.greet' → 'test-ext.greet' to avoid colliding with the + # primary command's SKILL output name (speckit.test-ext.hello → speckit-test-ext-hello). valid_manifest_data.pop("hooks", None) - valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.test-ext.hello"] + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.test-ext.greet"] manifest_path = temp_dir / "extension.yml" with open(manifest_path, 'w') as f: @@ -350,18 +356,34 @@ def test_alias_autocorrect_speckit_three_part(self, temp_dir, valid_manifest_dat manifest = ExtensionManifest(manifest_path) - assert manifest.commands[0]["aliases"] == ["test-ext.hello"] + assert manifest.commands[0]["aliases"] == ["test-ext.greet"] assert len(manifest.warnings) == 1 - assert "speckit.test-ext.hello" in manifest.warnings[0] - assert "test-ext.hello" in manifest.warnings[0] + assert "speckit.test-ext.greet" in manifest.warnings[0] + assert "test-ext.greet" in manifest.warnings[0] + + def test_alias_collision_with_primary_skill_name_rejected(self, temp_dir, valid_manifest_data): + """Alias whose SKILL output name matches the primary command's is rejected.""" + import yaml + + # Primary 'speckit.test-ext.hello' maps to skill 'speckit-test-ext-hello'. + # Alias 'test-ext.hello' would map to the same skill name — collision. + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.hello"] + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, "w") as f: + yaml.dump(valid_manifest_data, f) + + with pytest.raises(ValidationError, match="would collide with primary command"): + ExtensionManifest(manifest_path) def test_hook_alias_ref_canonicalized_to_speckit_form(self, temp_dir, valid_manifest_data): """Hook command refs in alias form are lifted to canonical speckit.ext.cmd form.""" import yaml - # Manifest uses a valid alias but the hook mistakenly references the alias name. - valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.hello"] - valid_manifest_data["hooks"]["after_tasks"]["command"] = "test-ext.hello" + # Manifest uses a valid alias (distinct suffix to avoid SKILL name collision) + # but the hook mistakenly references the alias name instead of the primary. + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["test-ext.greet"] + valid_manifest_data["hooks"]["after_tasks"]["command"] = "test-ext.greet" manifest_path = temp_dir / "extension.yml" with open(manifest_path, "w") as f: @@ -370,7 +392,7 @@ def test_hook_alias_ref_canonicalized_to_speckit_form(self, temp_dir, valid_mani manifest = ExtensionManifest(manifest_path) # Hook ref should be lifted to canonical form for skill-mode invocation. - assert manifest.hooks["after_tasks"]["command"] == "speckit.test-ext.hello" + assert manifest.hooks["after_tasks"]["command"] == "speckit.test-ext.greet" def test_hook_non_dict_value_raises(self, temp_dir, valid_manifest_data): """A non-mapping hooks value raises ValidationError.""" From ecd9c0aba37a74ccac6f04c1dcd7f04cf6f89958 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 20:08:50 +0200 Subject: [PATCH 13/16] fix(extensions): warn on alias hook canonicalization; fix pluralization (#2017) - Emit a compatibility warning when a hook ref is silently lifted from alias form (ext.cmd) to canonical (speckit.ext.cmd), so extension authors are notified to update their manifest in both rename and alias-canonicalization cases - Pluralize removal confirmation: '1 command' vs 'N commands from AI agent' - Update hook canonicalization test to assert the new warning is emitted - Fix test assertions to match correct singular/plural strings Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/__init__.py | 3 ++- src/specify_cli/extensions.py | 8 ++++++++ tests/test_extensions.py | 12 ++++++++---- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 49927e0b5d..65c5b21e76 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -3139,7 +3139,8 @@ def extension_remove( # Confirm removal if not force: console.print("\n[yellow]⚠ This will remove:[/yellow]") - console.print(f" • {cmd_count} commands from AI agent") + cmd_label = "command" if cmd_count == 1 else "commands" + console.print(f" • {cmd_count} {cmd_label} from AI agent") if skill_count: console.print(f" • {skill_count} agent skill(s)") console.print(f" • Extension directory: .specify/extensions/{extension_id}/") diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 9c94f788c0..7de6b33d4f 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -295,6 +295,14 @@ def _validate(self): f"updated to '{final_ref}'. " f"The extension author should update the manifest." ) + else: + # Alias-form ref was canonicalized (e.g. 'ext.cmd' → 'speckit.ext.cmd'). + self.warnings.append( + f"Hook '{hook_name}' referenced alias '{command_ref}'; " + f"updated to canonical command '{final_ref}'. " + f"The extension author should update the manifest to reference " + f"the canonical command name directly." + ) @staticmethod def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]: diff --git a/tests/test_extensions.py b/tests/test_extensions.py index c9ceb731df..d191cc3fc1 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -393,6 +393,10 @@ def test_hook_alias_ref_canonicalized_to_speckit_form(self, temp_dir, valid_mani # Hook ref should be lifted to canonical form for skill-mode invocation. assert manifest.hooks["after_tasks"]["command"] == "speckit.test-ext.greet" + # A compatibility warning should be emitted so the author knows to update. + assert len(manifest.warnings) == 1 + assert "test-ext.greet" in manifest.warnings[0] + assert "speckit.test-ext.greet" in manifest.warnings[0] def test_hook_non_dict_value_raises(self, temp_dir, valid_manifest_data): """A non-mapping hooks value raises ValidationError.""" @@ -3144,8 +3148,8 @@ def test_remove_confirmation_counts_primary_command_only(self, extension_dir, pr result = runner.invoke(app, ["extension", "remove", "test-ext"], input="n\n") plain = strip_ansi(result.output) - # The fixture has 1 command and 0 aliases → "1 commands from AI agent" - assert "1 commands from AI agent" in plain + # The fixture has 1 command and 0 aliases → "1 command from AI agent" + assert "1 command from AI agent" in plain def test_remove_confirmation_counts_aliases(self, temp_dir, project_dir): """Removal confirmation shows primary + alias count, not just primary.""" @@ -3198,9 +3202,9 @@ def test_remove_confirmation_counts_aliases(self, temp_dir, project_dir): result = runner.invoke(app, ["extension", "remove", "ext-with-alias"], input="n\n") plain = strip_ansi(result.output) - # 1 primary + 1 alias = "2 commands from AI agent" (not "1") + # 1 primary + 1 alias = "2 commands from AI agent" (not "1 command") assert "2 commands from AI agent" in plain - assert "1 commands from AI agent" not in plain + assert "1 command from AI agent" not in plain class TestExtensionUpdateCLI: From 9a48e78471911471777ff3ef407e58a2cde39249 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 21:18:44 +0200 Subject: [PATCH 14/16] fix(extensions): detect cross-command SKILL name collisions in manifest validation Extend the SKILL output name collision check in ExtensionManifest._validate() to cover cross-command scenarios: a primary on command A and an alias on command B that both map to the same SKILL.md directory (e.g. 'speckit.myext.build' and alias 'myext.build' on a different entry both produce 'speckit-myext-build/SKILL.md'). Previously only same-entry collisions were caught. Now a shared dict of seen SKILL output names is maintained across all commands and aliases, and any duplicate raises ValidationError with a clear message indicating the claimant. Adds _skill_output_name() static helper to mirror _compute_output_name logic for SKILL.md agents without importing the agents module. Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/extensions.py | 52 +++++++++++++++++++++++++++-------- tests/test_extensions.py | 23 +++++++++++++++- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 7de6b33d4f..00a87651c2 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -192,6 +192,12 @@ def _validate(self): # Validate commands; track renames so hook references can be rewritten. rename_map: Dict[str, str] = {} + # Track SKILL output names across all commands+aliases to catch cross-command + # collisions. For SKILL.md agents, _compute_output_name strips the 'speckit.' + # prefix and hyphenates: 'speckit.myext.build' and alias 'myext.build' on a + # *different* command entry both map to 'speckit-myext-build/SKILL.md'. + seen_skill_names: Dict[str, str] = {} # skill_output_name -> description of claimant + for cmd in provides["commands"]: if "name" not in cmd or "file" not in cmd: raise ValidationError("Command missing 'name' or 'file'") @@ -213,6 +219,17 @@ def _validate(self): "must follow pattern 'speckit.{extension}.{command}'" ) + # Register the primary command's SKILL output name. + primary_skill = self._skill_output_name(cmd["name"]) + if primary_skill in seen_skill_names: + raise ValidationError( + f"Command '{cmd['name']}' would produce SKILL output name " + f"'{primary_skill}' which is already claimed by " + f"{seen_skill_names[primary_skill]}. " + f"Choose a distinct command name." + ) + seen_skill_names[primary_skill] = f"primary command '{cmd['name']}'" + # Validate and auto-correct alias name formats aliases = cmd.get("aliases") if aliases is None: @@ -229,21 +246,22 @@ def _validate(self): ) alias_match = EXTENSION_ALIAS_PATTERN.match(alias) if alias_match and alias_match.group(1) != 'speckit': - # Valid 'ext.cmd' form — check it won't collide with the primary - # command's SKILL output name. For SKILL.md agents, - # _compute_output_name strips 'speckit.' so e.g. primary - # 'speckit.myext.run' and alias 'myext.run' both map to - # 'speckit-myext-run', causing the alias write to overwrite - # the primary skill file. - primary = cmd["name"] - if primary.startswith("speckit.") and primary[len("speckit."):] == alias: - skill_name = "speckit-" + alias.replace(".", "-") + # Valid 'ext.cmd' form — check it won't collide with any previously + # seen primary or alias SKILL output name (including its own command's + # primary). For SKILL.md agents, _compute_output_name strips 'speckit.' + # so e.g. primary 'speckit.myext.run' and alias 'myext.run' both map + # to 'speckit-myext-run', causing the alias write to overwrite the + # primary skill file. + alias_skill = self._skill_output_name(alias) + if alias_skill in seen_skill_names: + skill_name = alias_skill raise ValidationError( - f"Alias '{alias}' would collide with primary command " - f"'{primary}' on SKILL.md-based agents (both map to " - f"'{skill_name}'). " + f"Alias '{alias}' on command '{cmd['name']}' would produce " + f"SKILL output name '{skill_name}' which is already claimed by " + f"{seen_skill_names[alias_skill]}. " f"Choose a distinct alias name." ) + seen_skill_names[alias_skill] = f"alias '{alias}' on command '{cmd['name']}'" else: corrected = self._try_correct_alias_name(alias, ext["id"]) if corrected: @@ -304,6 +322,16 @@ def _validate(self): f"the canonical command name directly." ) + @staticmethod + def _skill_output_name(cmd_name: str) -> str: + """Compute the SKILL.md directory name for a command or alias. + + Mirrors the logic in CommandRegistrar._compute_output_name for SKILL.md + agents: strip the 'speckit.' prefix (if present) then hyphenate. + """ + short = cmd_name[len("speckit."):] if cmd_name.startswith("speckit.") else cmd_name + return "speckit-" + short.replace(".", "-") + @staticmethod def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]: """Try to auto-correct a non-conforming primary command name to 'speckit.{ext_id}.command'. diff --git a/tests/test_extensions.py b/tests/test_extensions.py index d191cc3fc1..076a601fb8 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -373,7 +373,28 @@ def test_alias_collision_with_primary_skill_name_rejected(self, temp_dir, valid_ with open(manifest_path, "w") as f: yaml.dump(valid_manifest_data, f) - with pytest.raises(ValidationError, match="would collide with primary command"): + with pytest.raises(ValidationError, match="would produce SKILL output name"): + ExtensionManifest(manifest_path) + + def test_cross_command_skill_name_collision_rejected(self, temp_dir, valid_manifest_data): + """Alias on one command that collides with a primary on a different command is rejected.""" + import yaml + + # Command A: primary 'speckit.test-ext.hello' → skill 'speckit-test-ext-hello' + # Command B: primary 'speckit.test-ext.build' with alias 'test-ext.hello' + # → alias also maps to 'speckit-test-ext-hello' — collision with cmd A. + valid_manifest_data["provides"]["commands"][0]["aliases"] = [] + valid_manifest_data["provides"]["commands"].append({ + "name": "speckit.test-ext.build", + "file": "commands/build.md", + "aliases": ["test-ext.hello"], + }) + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, "w") as f: + yaml.dump(valid_manifest_data, f) + + with pytest.raises(ValidationError, match="would produce SKILL output name"): ExtensionManifest(manifest_path) def test_hook_alias_ref_canonicalized_to_speckit_form(self, temp_dir, valid_manifest_data): From 426d28ec35b9078d62ed0864f392924914da35a8 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 21:41:35 +0200 Subject: [PATCH 15/16] fix(extensions): check SKILL collision for auto-corrected aliases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After auto-correcting an alias (e.g. 'speckit.test-ext.hello' → 'test-ext.hello'), the corrected name was written back to aliases[i] and rename_map but never checked against seen_skill_names nor added to it. This allowed a corrected alias to silently produce the same SKILL output name as an existing primary or alias. Now the corrected name is collision-checked before the warning is emitted and registered in seen_skill_names immediately after. Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/extensions.py | 10 ++++++++++ tests/test_extensions.py | 16 ++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 00a87651c2..e167fa4d05 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -265,6 +265,15 @@ def _validate(self): else: corrected = self._try_correct_alias_name(alias, ext["id"]) if corrected: + corrected_skill = self._skill_output_name(corrected) + if corrected_skill in seen_skill_names: + raise ValidationError( + f"Alias '{alias}' (corrected to '{corrected}') on command " + f"'{cmd['name']}' would produce SKILL output name " + f"'{corrected_skill}' which is already claimed by " + f"{seen_skill_names[corrected_skill]}. " + f"Choose a distinct alias name." + ) self.warnings.append( f"Alias '{alias}' does not follow the required pattern " f"'{{extension}}.{{command}}'. Registering as '{corrected}'. " @@ -272,6 +281,7 @@ def _validate(self): ) rename_map[alias] = corrected aliases[i] = corrected + seen_skill_names[corrected_skill] = f"alias '{corrected}' on command '{cmd['name']}'" else: raise ValidationError( f"Invalid alias '{alias}': " diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 076a601fb8..a9a995c954 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -397,6 +397,22 @@ def test_cross_command_skill_name_collision_rejected(self, temp_dir, valid_manif with pytest.raises(ValidationError, match="would produce SKILL output name"): ExtensionManifest(manifest_path) + def test_corrected_alias_skill_name_collision_rejected(self, temp_dir, valid_manifest_data): + """Auto-corrected alias that collides with an existing primary's SKILL name is rejected.""" + import yaml + + # Primary 'speckit.test-ext.hello' → skill 'speckit-test-ext-hello'. + # Alias 'speckit.test-ext.hello' gets auto-corrected to 'test-ext.hello' + # which also maps to 'speckit-test-ext-hello' — collision. + valid_manifest_data["provides"]["commands"][0]["aliases"] = ["speckit.test-ext.hello"] + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, "w") as f: + yaml.dump(valid_manifest_data, f) + + with pytest.raises(ValidationError, match="would produce SKILL output name"): + ExtensionManifest(manifest_path) + def test_hook_alias_ref_canonicalized_to_speckit_form(self, temp_dir, valid_manifest_data): """Hook command refs in alias form are lifted to canonical speckit.ext.cmd form.""" import yaml From b0019c5f43551cf64a9848f001a811b7c9a4def3 Mon Sep 17 00:00:00 2001 From: iamaeroplane Date: Sat, 4 Apr 2026 22:17:37 +0200 Subject: [PATCH 16/16] fix(extensions): raise ValidationError for non-dict hook entries; fix wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raise ValidationError instead of silently skipping hook entries that are not mappings. The silent skip allowed malformed manifests to pass validation and crash later in HookExecutor.register_hooks() with an AttributeError. Added test_hook_entry_non_dict_value_raises. Change 'from AI agent' to 'across AI agents' in the extension remove confirmation message — cmd_count is summed across all registered agents, so the singular 'agent' was misleading. Updated CLI tests accordingly. Co-Authored-By: Claude Sonnet 4.6 --- src/specify_cli/__init__.py | 2 +- src/specify_cli/extensions.py | 4 +++- tests/test_extensions.py | 25 ++++++++++++++++++++----- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 65c5b21e76..69f8ed4ee0 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -3140,7 +3140,7 @@ def extension_remove( if not force: console.print("\n[yellow]⚠ This will remove:[/yellow]") cmd_label = "command" if cmd_count == 1 else "commands" - console.print(f" • {cmd_count} {cmd_label} from AI agent") + console.print(f" • {cmd_count} {cmd_label} across AI agents") if skill_count: console.print(f" • {skill_count} agent skill(s)") console.print(f" • Extension directory: .specify/extensions/{extension_id}/") diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index e167fa4d05..4da238cbc3 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -299,7 +299,9 @@ def _validate(self): ) for hook_name, hook_data in hooks.items(): if not isinstance(hook_data, dict): - continue + raise ValidationError( + f"'hooks.{hook_name}' must be a mapping, got {type(hook_data).__name__}" + ) command_ref = hook_data.get("command") if not isinstance(command_ref, str): continue diff --git a/tests/test_extensions.py b/tests/test_extensions.py index a9a995c954..22b88574c2 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -448,6 +448,21 @@ def test_hook_non_dict_value_raises(self, temp_dir, valid_manifest_data): with pytest.raises(ValidationError, match="'hooks' must be a mapping"): ExtensionManifest(manifest_path) + def test_hook_entry_non_dict_value_raises(self, temp_dir, valid_manifest_data): + """A hook entry that is not a mapping raises ValidationError instead of silently skipping.""" + import yaml + + # hooks.after_tasks is a list instead of a dict — should fail validation, + # not silently pass and crash later in HookExecutor. + valid_manifest_data["hooks"]["after_tasks"] = ["x", "y"] + + manifest_path = temp_dir / "extension.yml" + with open(manifest_path, "w") as f: + yaml.dump(valid_manifest_data, f) + + with pytest.raises(ValidationError, match="'hooks.after_tasks' must be a mapping"): + ExtensionManifest(manifest_path) + def test_valid_command_name_has_no_warnings(self, temp_dir, valid_manifest_data): """Test that a correctly-named command produces no warnings.""" import yaml @@ -3185,8 +3200,8 @@ def test_remove_confirmation_counts_primary_command_only(self, extension_dir, pr result = runner.invoke(app, ["extension", "remove", "test-ext"], input="n\n") plain = strip_ansi(result.output) - # The fixture has 1 command and 0 aliases → "1 command from AI agent" - assert "1 command from AI agent" in plain + # The fixture has 1 command and 0 aliases → "1 command across AI agents" + assert "1 command across AI agents" in plain def test_remove_confirmation_counts_aliases(self, temp_dir, project_dir): """Removal confirmation shows primary + alias count, not just primary.""" @@ -3239,9 +3254,9 @@ def test_remove_confirmation_counts_aliases(self, temp_dir, project_dir): result = runner.invoke(app, ["extension", "remove", "ext-with-alias"], input="n\n") plain = strip_ansi(result.output) - # 1 primary + 1 alias = "2 commands from AI agent" (not "1 command") - assert "2 commands from AI agent" in plain - assert "1 command from AI agent" not in plain + # 1 primary + 1 alias = "2 commands across AI agents" (not "1 command") + assert "2 commands across AI agents" in plain + assert "1 command across AI agents" not in plain class TestExtensionUpdateCLI: