fix: unofficial PyPI warning (#1982) and legacy extension command name auto-correction (#2017)#2027
Conversation
…ication (github#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>
There was a problem hiding this comment.
Pull request overview
This PR improves user safety and extension compatibility by warning against unofficial PyPI packages, adding a post-install verification step, and making legacy community extension command names auto-correct (with warnings) instead of hard-failing. It also stabilizes a previously flaky preset-catalog test by preventing unintended live community catalog lookups.
Changes:
- Add prominent documentation warnings that Spec Kit’s official packages are only distributed from
github/spec-kit, plus a recommendedspecify versionverification step. - In extension manifest validation, auto-correct common legacy 2-part command names to the required 3-part
speckit.{extension}.{command}form and surface compatibility warnings on install. - Make
TestPresetCatalog::test_search_with_cached_datadeterministic by restricting catalogs to default-only via a project-levelpreset-catalogs.yml.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
README.md |
Adds an “official source only” warning and a specify version verification step after install instructions. |
docs/installation.md |
Adds the same warning and a verification section recommending specify version. |
src/specify_cli/extensions.py |
Adds command-name regex constant, manifest warnings, and auto-correction logic for legacy command names. |
src/specify_cli/__init__.py |
Prints manifest compatibility warnings after successful specify extension add. |
tests/test_extensions.py |
Adds tests for both auto-correction paths and the no-warning valid path; updates an existing docstring. |
tests/test_presets.py |
Writes .specify/preset-catalogs.yml in a test to avoid community-catalog network flakiness. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…iling (github#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>
… 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>
mbachorik
force-pushed
the
fix/pypi-warning-and-legacy-extension-names
branch
from
56c6b66 to
44d1996
Compare
The upstream github#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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. If not applicable, please explain why
- _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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
|
@mnriem I've bundled this with the fix for 2/3dot alias notation. Will wait for feedback from the reporter (or we decide tonight and agree on fix). Will address all copilot feedback as usual .. (switching it to draft so it does not spam you) |
mbachorik
force-pushed
the
fix/pypi-warning-and-legacy-extension-names
branch
from
3fed7f7 to
caee3e0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ed; 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>
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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@mnriem happy easter. ready again. |
There was a problem hiding this comment.
Pull request overview
This PR improves installation safety messaging (warns about unofficial PyPI packages) and restores compatibility for community extensions by auto-correcting legacy command/alias names while emitting warnings instead of hard-failing, plus it stabilizes a previously flaky preset-catalog test.
Changes:
- Add prominent GitHub-only package warnings and a
specify versionverification step to install docs. - Extend extension manifest validation to auto-correct certain legacy command/alias names and rewrite hook command references with compatibility warnings.
- Update CLI UX for extension install/remove output and patch a preset-catalog test to avoid live network calls.
Show a summary per file
| File | Description |
|---|---|
README.md |
Adds official-source warning + post-install specify version verification step. |
docs/installation.md |
Adds GitHub-only warning and verification guidance. |
src/specify_cli/extensions.py |
Implements command/alias auto-correction + hook reference rewriting + warning accumulation. |
src/specify_cli/__init__.py |
Prints manifest compatibility warnings on install; improves extension-remove command count text. |
tests/test_extensions.py |
Adds coverage for auto-correction and hook validation behavior changes. |
tests/test_presets.py |
Removes env override and patches catalog selection to prevent network flakiness. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 6/6 changed files
- Comments generated: 1
mnriem
left a comment
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
|
Fixed the Changed "across AI agents" → "per agent" — this accurately describes what |
'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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Fixed the PR description discrepancy (comment #3064573350). The
So collision protection already exists; the PR description was overstating what |
Upstream added hook-only extension support to ExtensionManifest._validate(): type checks for provides.commands and hooks, allows extensions with only hooks (no commands), and validates individual hook entries have a 'command' field. Merged with our rename_map / command auto-correction logic. Test conflicts: upstream renamed test_no_commands → test_no_commands_no_hooks and softened the alias autocorrect assertion. Kept our detailed assertions (they test the actual behavior this PR introduces) and applied the rename. Updated hook error message match to upstream's new wording. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Derive cmd_count from the registry's registered_commands (includes aliases, | ||
| # covers all agents) rather than from the manifest (primary commands only). | ||
| # Use get() without a default so we can distinguish "key missing" (fall back | ||
| # to manifest) from "key present but empty dict" (zero commands registered). | ||
| registered_commands = reg_meta.get("registered_commands") if isinstance(reg_meta, dict) else None | ||
| if isinstance(registered_commands, dict): | ||
| cmd_count = max( | ||
| (len(v) for v in registered_commands.values() if isinstance(v, list)), | ||
| default=0, | ||
| ) |
There was a problem hiding this comment.
The inline comment says cmd_count derivation “covers all agents”, but the logic intentionally uses max(...) and the prompt says “per agent”. Consider updating the comment to reflect that this is a per-agent count (max across agents) to avoid future maintainers misinterpreting it as a total across agents.
| # Validate commands; track renames so hook references can be rewritten. | ||
| rename_map: Dict[str, str] = {} | ||
| for cmd in commands: | ||
| if "name" not in cmd or "file" not in cmd: | ||
| raise ValidationError("Command missing 'name' or 'file'") | ||
|
|
||
| # Validate command name format | ||
| if EXTENSION_COMMAND_NAME_PATTERN.match(cmd["name"]) is None: | ||
| 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." | ||
| ) | ||
| rename_map[cmd["name"]] = corrected | ||
| cmd["name"] = corrected | ||
| else: | ||
| raise ValidationError( | ||
| f"Invalid command name '{cmd['name']}': " | ||
| "must follow pattern 'speckit.{extension}.{command}'" | ||
| ) | ||
|
|
||
| # Validate and auto-correct alias name formats | ||
| 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: | ||
| 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." | ||
| ) | ||
| rename_map[alias] = corrected | ||
| aliases[i] = corrected | ||
| else: | ||
| raise ValidationError( | ||
| f"Invalid alias '{alias}': " | ||
| "must follow pattern 'speckit.{extension}.{command}'" | ||
| ) | ||
|
|
There was a problem hiding this comment.
PR description mentions SKILL output-name collision detection via a seen_skill_names map across commands/aliases, but this _validate() implementation doesn’t perform any output-name collision checking (only pattern/alias correction). If collision detection is still required, it needs to be implemented here; otherwise, update the PR description to avoid documenting behavior that isn’t shipped.
Summary
Changes
`docs/installation.md` / `README.md`
`src/specify_cli/extensions.py`
`src/specify_cli/init.py`
`src/specify_cli/agents.py`
`tests/test_extensions.py`
`tests/test_presets.py`
Test plan
Unit tests
pytest tests/test_extensions.py)Live integration tests (real CLI, real filesystem, temp project dir)
All scenarios run against the workspace CLI with actual
specify extension add/removeinvocations:Well-formed extension (foobar test extension):
Legacy 2-part command name (
foobar.hello→speckit.foobar.hello):Hook with alias-form ref (
foobar.hello→speckit.foobar.hello):Non-dict hook entry (install must fail):
Remove — singular (
1 command):Remove — plural (
2 commands):🤖 Generated with Claude Code