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.
| # Validate command name format | ||
| if not re.match(r'^speckit\.[a-z0-9-]+\.[a-z0-9-]+$', cmd["name"]): | ||
| raise ValidationError( | ||
| f"Invalid command name '{cmd['name']}': " | ||
| "must follow pattern 'speckit.{extension}.{command}'" | ||
| ) | ||
| if not self._COMMAND_NAME_RE.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 |
There was a problem hiding this comment.
Auto-correcting provides.commands[*].name can leave other manifest fields (notably hooks.*.command) pointing at the old, now-unregistered command name. That would install successfully but register hooks that later try to execute a non-existent command. Consider tracking an old→new name mapping during validation and rewriting any hook command values (and/or validating they reference an existing provided command), emitting an additional warning when a hook reference is updated.
| # 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" | ||
| ) |
There was a problem hiding this comment.
This test restricts catalogs by writing .specify/preset-catalogs.yml, but PresetCatalog.get_active_catalogs() will still ignore that file if SPECKIT_PRESET_CATALOG_URL is set in the environment (env var has highest precedence). That can reintroduce non-determinism / network calls in some CI or developer environments. Consider explicitly unsetting SPECKIT_PRESET_CATALOG_URL in this test via monkeypatch.delenv(..., raising=False) (and add monkeypatch to the test signature).
…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.
| - '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]}" |
There was a problem hiding this comment.
_try_correct_command_name() builds the legacy extension.command correction as speckit.{parts[0]}.{parts[1]}. That can produce a command namespace that doesn’t match ext_id, but install-time validation later requires the namespace to equal manifest.id (ExtensionManager._collect_manifest_command_names). In that mismatch case, the manifest validator will emit a warning claiming it’s “Registering as …” even though install will fail. Consider only applying this correction when parts[0] == ext_id (or always using ext_id for the namespace) so the auto-correction is guaranteed to produce an installable name and the warning remains accurate.
| - '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]}" | |
| - 'extension.command' → 'speckit.{ext_id}.command' (when extension == ext_id) | |
| Returns the corrected name, or None if no safe correction is possible. | |
| """ | |
| parts = name.split('.') | |
| if len(parts) == 2: | |
| if parts[0] == 'speckit': | |
| # Legacy form: 'speckit.command' → 'speckit.{ext_id}.command' | |
| candidate = f"speckit.{ext_id}.{parts[1]}" | |
| elif parts[0] == ext_id: | |
| # Legacy form: '{ext_id}.command' → 'speckit.{ext_id}.command' | |
| candidate = f"speckit.{ext_id}.{parts[1]}" | |
| else: | |
| # Namespace does not match ext_id; auto-correction would produce | |
| # a name that fails install-time validation, so do not correct. | |
| return None |
| # 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: |
There was a problem hiding this comment.
aliases = cmd.get("aliases") or [] is assumed to be a mutable list, but YAML (or programmatic callers) could provide a non-list value (e.g., a single string). In that case, enumerate(aliases) will iterate characters or aliases[i] = corrected could raise (e.g., tuple), leading to confusing errors or an unhandled exception instead of a ValidationError. Add a type check here (similar to ExtensionManager._collect_manifest_command_names) and raise a clear ValidationError if aliases is not a list of strings before attempting auto-correction.
| 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] | ||
|
|
There was a problem hiding this comment.
test_command_name_autocorrect_no_speckit_prefix uses valid_manifest_data with extension.id == "test-ext" but expects a corrected name under a different namespace (speckit.docguard.guard). Since install-time validation requires command/alias namespaces to match manifest.id, this is not a correction that would be installable for the given manifest. Consider adjusting the test data so the manifest extension.id matches the legacy namespace being corrected (or asserting the behavior when they differ), to better reflect the real compatibility path and avoid masking namespace-mismatch failures.
Summary
README.mdanddocs/installation.mdthat the only official packages are published fromgithub/spec-kit; any same-named PyPI packages are unaffiliated. Also addsspecify versionas a recommended post-install verification step.docguard.guardorspeckit.verify), auto-correct it to the requiredspeckit.{extension}.{command}pattern and emit a compatibility warning so authors know to update their manifest. Names that cannot be safely corrected still raiseValidationError.TestPresetCatalog::test_search_with_cached_data) that was making live network calls to the community catalog, causing non-deterministic result counts.Changes
docs/installation.md/README.mdspecify versionverification step added after install instructionssrc/specify_cli/extensions.pyExtensionManifest.__init__: initialisesself.warnings: List[str]ExtensionManifest._validate(): on invalid command name, attempts auto-correction before raisingExtensionManifest._try_correct_command_name(): new static method handlingX.Y → speckit.X.Yandspeckit.Y → speckit.{ext_id}.Y_COMMAND_NAME_RE: compiled class-level regex constant (replaces two rawre.matchcalls)src/specify_cli/__init__.pyextension_add: prints compatibility warnings frommanifest.warningsafter successful installtests/test_extensions.pytests/test_presets.pytest_search_with_cached_data: writes a project-levelpreset-catalogs.ymlto restrict the test to the default catalog only, preventing live community catalog network callsTest plan
specify extension add --from <legacy-zip>installs with a compatibility warning instead of failingspecify extension add --from <valid-zip>installs silently (no warning)invalid-name) still raiseValidationError🤖 Generated with Claude Code