github/github-spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-07-03 12:28:06 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix: unofficial PyPI warning (#1982) and legacy extension command name auto-correction (#2017) (#2027)

Browse Source 
* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: iamaeroplane <michal.bachorik@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Michal Bachorik
2026-04-15 14:35:49 +02:00
committed by GitHub
parent f0886bd089
commit 33a28ec8f7
7 changed files with 334 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

205
tests/test_extensions.py
View File

@@ -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: `/<missing command>`" in message
assert "EXECUTE_COMMAND: <missing command>" in message
assert "EXECUTE_COMMAND_INVOCATION: /<missing command>" 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

3
tests/test_presets.py
View File

@@ -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)