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>
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
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
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) |
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.
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>
|
Fixed (#3067647278). Implemented the cleanup in After unlinking |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 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.
|
All comments addressed. |
|
Are we still supporting the original aliasing? We need to give users time to migrate especially since it break extensions. I already reverted that part in an earlier PR, but I do not want to reintroduce the hard change now. We should reserve that for a later date. |
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, github#2110/github#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>
|
Fixed. Removed alias pattern enforcement from Verified end-to-end: a |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 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
There was a problem hiding this comment.
Please address Copilot feedback. Thank you for staying with it!
There was a problem hiding this comment.
Pull request overview
This PR improves user guidance around unofficial PyPI packages and increases backward compatibility for community extensions by auto-correcting certain legacy command names, while also tightening/covering related behaviors with additional tests.
Changes:
- Add documentation warnings that the official Spec Kit packages are sourced from
github/spec-kit, plus a post-installspecify versionverification step. - Extend extension manifest validation to auto-correct legacy 2-part primary command names and rewrite hook command references, emitting compatibility warnings.
- Improve uninstall cleanup (remove empty SKILL parent directories) and add/adjust tests, including isolating a previously flaky preset catalog test from network access.
Show a summary per file
| File | Description |
|---|---|
docs/installation.md |
Adds prominent warning about unofficial PyPI packages and adds a verification step. |
README.md |
Adds the same warning + verification step to the main install instructions. |
src/specify_cli/extensions.py |
Adds manifest.warnings, legacy command auto-correction, and hook command ref rewriting. |
src/specify_cli/__init__.py |
Prints compatibility warnings after extension install and fixes remove confirmation pluralization. |
src/specify_cli/agents.py |
Removes empty SKILL parent directories after unregistering commands. |
tests/test_extensions.py |
Adds coverage for command auto-correction, hook rewriting, SKILL dir cleanup, and removal prompt wording. |
tests/test_presets.py |
Ensures cached-search test is deterministic and isolated from live network catalogs. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 1
|
@mnriem should ok now - there's no additional fix. |
|
@mbachorik Looks like it found one! |
|
it's like a Russian roulette .. looking (before I crash for tonight) |
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>
|
Fixed comment #3082042943. Added |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 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 no new comments from copilot (at least for me). |
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