From 33a28ec8f7f87b3facce51a6226581683aaa05e1 Mon Sep 17 00:00:00 2001 From: Michal Bachorik Date: Wed, 15 Apr 2026 14:35:49 +0200 Subject: [PATCH] fix: unofficial PyPI warning (#1982) and legacy extension command name auto-correction (#2017) (#2027) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 * 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 * 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 * 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 * Update docs/installation.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(extensions): warn when hook command refs are silently canonicalized; fix grammar - Hook rewrites (alias-form or rename-map) now always emit a warning so extension authors know to update their manifests. Previously only rename-map rewrites produced a warning; pure alias-form lifts were silent. - Pluralize "command/commands" in the uninstall confirmation message so single-command extensions no longer print "1 commands". Co-Authored-By: Claude Sonnet 4.6 * fix(extensions): raise ValidationError for non-dict hook entries Silently skipping non-dict hook entries left them in manifest.hooks, causing HookExecutor.register_hooks() to crash with AttributeError when it called hook_config.get() on a non-mapping value. Also updates PR description to accurately reflect the implementation (no separate _try_correct_alias_name helper; aliases use the same _try_correct_command_name path). Co-Authored-By: Claude Sonnet 4.6 * fix(extensions): derive remove cmd_count from registry, fix wording Previously cmd_count used len(ext_manifest.commands) which only counted primary commands and missed aliases. The registry's registered_commands already tracks every command name (primaries + aliases) per agent, so max(len(v) for v in registered_commands.values()) gives the correct total. Also changes "from AI agent" → "across AI agents" since remove() unregisters commands from all detected agents. Co-Authored-By: Claude Sonnet 4.6 * fix(extensions): distinguish missing vs empty registered_commands in remove prompt Using get() without a default lets us tell apart: - key missing (legacy registry entry) → fall back to manifest count - key present but empty dict (installed with no agent dirs) → show 0 Previously the truthiness check `if registered_commands and ...` treated both cases the same, so an empty dict fell back to len(manifest.commands) and overcounted commands that would actually be removed. Co-Authored-By: Claude Sonnet 4.6 * fix(extensions): clarify removal prompt wording to 'per agent' 'across AI agents' implied a total count, but cmd_count uses max() across agents (per-agent count). Using sum() would double-count since users think in logical commands, not per-agent files. 'per agent' accurately describes what the number represents. Co-Authored-By: Claude Sonnet 4.6 * fix(extensions): clarify cmd_count comment — per-agent max, not total The comment said 'covers all agents' implying a total, but cmd_count uses max() across agents (per-agent count). Updated comment to explain the max() choice and why sum() would double-count. Co-Authored-By: Claude Sonnet 4.6 * test(extensions): add CLI tests for remove confirmation pluralization Adds TestExtensionRemoveCLI with two CliRunner tests: - singular: 1 registered command → '1 command per agent' - plural: 2 registered commands → '2 commands per agent' These prevent regressions on the cmd_count pluralization logic and the 'per agent' wording introduced in this PR. Co-Authored-By: Claude Sonnet 4.6 * fix(agents): remove orphaned SKILL.md parent dirs on unregister For SKILL.md-based agents (codex, kimi), each command lives in its own subdirectory (e.g. .agents/skills/speckit-ext-cmd/SKILL.md). The previous unregister_commands() only unlinked the file, leaving an empty parent dir. Now attempts rmdir() on the parent when it differs from the agent commands dir. OSError is silenced so non-empty dirs (e.g. user files) are safely left. Adds test_unregister_skill_removes_parent_directory to cover this. Co-Authored-By: Claude Sonnet 4.6 * fix(extensions): drop alias pattern enforcement from _validate() Aliases are intentionally free-form to preserve community extension compatibility (e.g. 'speckit.verify' short aliases used by spec-kit-verify and other existing extensions). This aligns _validate() with the intent of upstream commit 4deb90f (fix: restore alias compatibility, #2110/#2125). Only type and None-normalization checks remain for aliases. Pattern enforcement continues for primary command names only. Updated tests to verify free-form aliases pass through unchanged with no warnings instead of being auto-corrected. Co-Authored-By: Claude Sonnet 4.6 * fix(extensions): guard against non-dict command entries in _validate() If provides.commands contains a non-mapping entry (e.g. an int or string), 'name' not in cmd raises TypeError instead of a user-facing ValidationError. Added isinstance(cmd, dict) check at the top of the loop. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: iamaeroplane Co-authored-by: Claude Sonnet 4.6 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- README.md | 10 +- docs/installation.md | 10 ++ src/specify_cli/__init__.py | 21 +++- src/specify_cli/agents.py | 9 ++ src/specify_cli/extensions.py | 90 ++++++++++++++- tests/test_extensions.py | 205 +++++++++++++++++++++++++++++++++- tests/test_presets.py | 3 +- 7 files changed, 334 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index a8a8d3a3e..54e245d63 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 5d560b6e3..ed253902a 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -10,6 +10,8 @@ ## Installation +> **Important:** The only official, maintained packages for Spec Kit come 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. For normal installs, use the GitHub-based commands shown below. For offline or air-gapped environments, locally built wheels created from this repository are also valid. + ### 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 Optional[str]: + """Try to auto-correct a non-conforming command name to the required pattern. + + 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' or parts[0] == ext_id: + candidate = f"speckit.{ext_id}.{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 a6ddff8e1..bec939702 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -243,7 +243,7 @@ class TestExtensionManifest: 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" @@ -255,6 +255,83 @@ class TestExtensionManifest: 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_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" + 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_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_free_form_accepted(self, temp_dir, valid_manifest_data): + """Aliases are free-form — a 'speckit.command' alias must be accepted unchanged.""" + 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.hello"] + assert manifest.warnings == [] + + 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_no_hooks(self, temp_dir, valid_manifest_data): """Test manifest with no commands and no hooks provided.""" import yaml @@ -317,6 +394,19 @@ class TestExtensionManifest: with pytest.raises(ValidationError, match="Invalid hooks"): ExtensionManifest(manifest_path) + def test_non_dict_hook_entry_raises_validation_error(self, temp_dir, valid_manifest_data): + """Non-mapping hook entries must raise ValidationError, not silently skip.""" + import yaml + + valid_manifest_data["hooks"]["after_tasks"] = "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="Invalid hook 'after_tasks'"): + ExtensionManifest(manifest_path) + def test_manifest_hash(self, extension_dir): """Test manifest hash calculation.""" manifest_path = extension_dir / "extension.yml" @@ -686,8 +776,8 @@ class TestExtensionManager: 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_accepts_short_alias(self, temp_dir, project_dir): - """Install should accept legacy short aliases for community extension compat.""" + def test_install_accepts_free_form_alias(self, temp_dir, project_dir): + """Aliases are free-form — a short 'speckit.shortcut' alias must be preserved unchanged.""" import yaml ext_dir = temp_dir / "alias-shortcut" @@ -718,8 +808,10 @@ class TestExtensionManager: (ext_dir / "commands" / "cmd.md").write_text("---\ndescription: Test\n---\n\nBody") manager = ExtensionManager(project_dir) - # Should not raise — short aliases are allowed - 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.shortcut"] + assert manifest.warnings == [] def test_install_rejects_namespace_squatting(self, temp_dir, project_dir): """Install should reject commands and aliases outside the extension namespace.""" @@ -1619,6 +1711,54 @@ Then {AGENT_SCRIPT} prompts_dir = project_dir / ".github" / "prompts" assert not prompts_dir.exists() + def test_unregister_skill_removes_parent_directory(self, project_dir, temp_dir): + """Unregistering a SKILL.md command should remove the empty parent subdirectory.""" + import yaml + + ext_dir = temp_dir / "cleanup-ext" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "cleanup-ext", + "name": "Cleanup Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.cleanup-ext.run", + "file": "commands/run.md", + "description": "Run", + } + ] + }, + } + with open(ext_dir / "extension.yml", "w") as f: + yaml.dump(manifest_data, f) + (ext_dir / "commands" / "run.md").write_text("---\ndescription: Run\n---\n\nBody") + + skills_dir = project_dir / ".agents" / "skills" + skills_dir.mkdir(parents=True) + + registrar = CommandRegistrar() + from specify_cli.extensions import ExtensionManifest + manifest = ExtensionManifest(ext_dir / "extension.yml") + registered = registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir) + + skill_subdir = skills_dir / "speckit-cleanup-ext-run" + assert skill_subdir.exists(), "Skill subdirectory should exist after registration" + assert (skill_subdir / "SKILL.md").exists() + + registrar.unregister_commands({"codex": ["speckit.cleanup-ext.run"]}, project_dir) + + assert not (skill_subdir / "SKILL.md").exists(), "SKILL.md should be removed" + assert not skill_subdir.exists(), "Empty parent subdirectory should be removed" + # ===== Utility Function Tests ===== @@ -3853,3 +3993,58 @@ class TestHookInvocationRendering: assert "Executing: `/`" in message assert "EXECUTE_COMMAND: " in message assert "EXECUTE_COMMAND_INVOCATION: /" in message + + +class TestExtensionRemoveCLI: + """CLI tests for `specify extension remove` confirmation prompt wording.""" + + def _install_ext(self, project_dir, ext_dir): + """Install extension and return the manager.""" + manager = ExtensionManager(project_dir) + manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + return manager + + def test_remove_confirmation_singular_command(self, tmp_path, extension_dir): + """Confirmation prompt should say '1 command' (singular) when one command registered.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + + manager = self._install_ext(project_dir, extension_dir) + # Inject registered_commands with 1 entry so cmd_count == 1 + manager.registry.update("test-ext", {"registered_commands": {"claude": ["speckit.test-ext.hello"]}}) + + runner = CliRunner() + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke( + app, ["extension", "remove", "test-ext"], input="n\n", catch_exceptions=False + ) + + assert "1 command" in result.output + assert "1 commands" not in result.output + + def test_remove_confirmation_plural_commands(self, tmp_path, extension_dir): + """Confirmation prompt should say '2 commands' (plural) when two commands registered.""" + from typer.testing import CliRunner + from unittest.mock import patch + from specify_cli import app + + project_dir = tmp_path / "project" + project_dir.mkdir() + (project_dir / ".specify").mkdir() + + manager = self._install_ext(project_dir, extension_dir) + # Inject registered_commands with 2 entries so cmd_count == 2 + manager.registry.update("test-ext", {"registered_commands": {"claude": ["speckit.test-ext.hello", "speckit.test-ext.run"]}}) + + runner = CliRunner() + with patch.object(Path, "cwd", return_value=project_dir): + result = runner.invoke( + app, ["extension", "remove", "test-ext"], input="n\n", catch_exceptions=False + ) + + assert "2 commands" in result.output diff --git a/tests/test_presets.py b/tests/test_presets.py index 95af7a900..c7383a1f4 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1175,8 +1175,7 @@ class TestPresetCatalog: """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)