feat(extensions): scripts support, command filtering, and template discovery#1964
feat(extensions): scripts support, command filtering, and template discovery#1964mbachorik wants to merge 4 commits intogithub:mainfrom
Conversation
…e discovery Bring extension system to parity with presets across three areas: - github#1847: Extensions now support `provides.scripts` alongside commands, with name format and path safety validation, executable permissions, and CLI display in list/add/info. - github#1848: Add `_filter_commands_for_installed_extensions()` to CommandRegistrar for cross-boundary command filtering (matching the preset filtering at presets.py:518-529). - github#1846: Add `list_available()` to PresetResolver for template discovery across the 4-tier priority stack with source attribution, and a new `specify preset list-templates` CLI command. Closes github#1846, closes github#1847, closes github#1848 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends Spec Kit’s extension and preset ecosystems to improve parity and discoverability: extensions can now declare scripts, commands can be filtered based on installed extensions, and presets gain a resolver-backed template discovery/listing command.
Changes:
- Add extension manifest + manager support for
provides.scripts, including validation, install-time.shexecutable bits, and CLI surface area (list/add/info). - Introduce
CommandRegistrar._filter_commands_for_installed_extensions()plus tests to filterspeckit.<ext-id>.<cmd>commands when the target extension isn’t installed. - Add
PresetResolver.list_available()and a newspecify preset list-templates --type <...>command, with template discovery tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/extensions.py |
Adds scripts support to extension manifests/installs and introduces command filtering utility. |
src/specify_cli/presets.py |
Adds PresetResolver.list_available() for priority-stack template discovery with sources. |
src/specify_cli/__init__.py |
Adds preset list-templates CLI command and surfaces script counts in extension CLI output. |
tests/test_extensions.py |
Updates manifest validation tests and adds coverage for scripts support + command filtering utility. |
tests/test_presets.py |
Adds tests for PresetResolver.list_available() behavior (priority, sources, sorting). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Determine file extension and subdirectory mapping | ||
| ext = ".sh" if template_type == "script" else ".md" | ||
| if template_type == "template": | ||
| subdirs = ["templates", ""] | ||
| elif template_type == "command": | ||
| subdirs = ["commands"] | ||
| elif template_type == "script": | ||
| subdirs = ["scripts"] | ||
| else: | ||
| subdirs = [""] | ||
|
|
There was a problem hiding this comment.
list_available() falls back to scanning the pack/extension root for unknown template_type values (via the else: subdirs = [""] branch) and will still use .md vs .sh heuristics. Since this is a public resolver API (and is now exposed via CLI), consider rejecting unsupported template types early (e.g., raise ValueError or return an empty list) to avoid confusing output and accidental exposure of unrelated .md files as “templates”.
There was a problem hiding this comment.
Fixed in d6e0773 — list_available() now raises ValueError for unsupported types, and the else fallback branch is removed.
src/specify_cli/extensions.py
Outdated
| # Validate file path safety: must be relative, no parent traversal | ||
| file_path = script["file"] | ||
| normalized = os.path.normpath(file_path) | ||
| if os.path.isabs(normalized) or normalized.startswith(".."): | ||
| raise ValidationError( | ||
| f"Invalid script file path '{file_path}': " | ||
| "must be a relative path within the extension directory" | ||
| ) |
There was a problem hiding this comment.
The script path safety check is incomplete on Windows: os.path.isabs() does not treat drive-relative paths like C:foo\bar as absolute, but Path(dest_dir) / "C:foo" will ignore dest_dir and can point outside the extension directory. Consider validating with Path(file_path) (or PurePath) to reject any anchored paths (drive/UNC/root) and any .. path components, rather than relying on normpath(...).startswith("..").
There was a problem hiding this comment.
Fixed in d6e0773 — now uses Path.anchor to catch drive-relative/UNC paths and checks '..' in p.parts instead of relying on normpath.
src/specify_cli/extensions.py
Outdated
| if script_path.exists() and script_path.suffix == ".sh": | ||
| script_path.chmod(script_path.stat().st_mode | 0o755) |
There was a problem hiding this comment.
chmod(... | 0o755) sets read/execute bits broadly (and may unexpectedly change readability), not just “make executable”. If the goal is only to add execute bits, consider OR-ing with 0o111 (or at least 0o100) and gating the chmod to POSIX platforms to avoid confusing behavior on Windows filesystems.
| if script_path.exists() and script_path.suffix == ".sh": | |
| script_path.chmod(script_path.stat().st_mode | 0o755) | |
| if os.name == "posix" and script_path.exists() and script_path.suffix == ".sh": | |
| # Only add execute bits; preserve existing read/write permissions | |
| script_path.chmod(script_path.stat().st_mode | 0o111) |
There was a problem hiding this comment.
Fixed in d6e0773 — gated to os.name == "posix" and uses 0o111 (execute-only bits).
| extensions_dir = project_root / ".specify" / "extensions" | ||
| if not extensions_dir.is_dir(): | ||
| return commands | ||
| filtered = [] |
There was a problem hiding this comment.
The filter currently returns the input unmodified when .specify/extensions/ doesn’t exist. That makes extension-scoped commands (speckit.<ext>.<cmd>) appear even though no extensions can be installed/recognized yet, and it doesn’t mirror the existing preset filtering logic (presets.py:518-529) which implicitly treats a missing extensions dir as “no extensions installed”. Consider treating a missing extensions dir as empty and filtering out extension-scoped commands in that case (and update the docstring accordingly).
There was a problem hiding this comment.
Fixed in d6e0773 — missing extensions dir is now treated as empty (all extension-scoped commands filtered out), matching preset behavior.
| def preset_list_templates( | ||
| template_type: str = typer.Option( | ||
| "template", "--type", "-t", | ||
| help="Template type: template, command, or script", | ||
| ), | ||
| ): |
There was a problem hiding this comment.
preset list-templates --type accepts any string, but PresetResolver.list_available() only meaningfully supports "template", "command", and "script". Consider validating template_type (e.g., with typer.Choice([...])/Enum) and exiting with a clear error for invalid values, instead of silently producing surprising results.
There was a problem hiding this comment.
Fixed in d6e0773 — CLI validates --type against VALID_PRESET_TEMPLATE_TYPES with a clear error before calling the resolver.
Move extension template resolution and discovery into a dedicated ExtensionResolver class in extensions.py. PresetResolver now delegates its tier-3 (extension) lookups to ExtensionResolver instead of walking extension directories directly. This gives extensions their own resolution/discovery API without coupling them to the preset system. PresetResolver remains the unified resolver across all 4 tiers but no longer owns extension-specific logic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use Path.anchor to reject drive-relative/UNC paths in script validation, not just os.path.isabs + normpath - chmod only adds execute bits (0o111) and is gated to POSIX - Command filter treats missing extensions dir as empty (filters out all extension-scoped commands), matching preset behavior - list_available() rejects unsupported template_type with ValueError - CLI list-templates validates --type before calling resolver Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Priority 1: Project-local overrides | ||
| if template_type == "script": | ||
| _collect(self.overrides_dir / "scripts", "project override") | ||
| else: | ||
| _collect(self.overrides_dir, "project override") |
There was a problem hiding this comment.
list_available() scans .specify/templates/overrides/ for any *.md when template_type is template or command, but that overrides directory is shared by both template and command overrides. As a result, list_available('template') can incorrectly include command overrides (names with dots like speckit.*) and list_available('command') can include non-command templates. Filter override entries by expected name format (e.g., commands must match the command-name pattern / contain dots; templates must match the hyphen-only pattern) so discovery output reflects the selected template_type.
There was a problem hiding this comment.
Fixed in aa1df03 — list_available() now filters override entries by name pattern: commands must contain dots (e.g. speckit.specify), templates must not (e.g. spec-template). Scripts are already in their own subdirectory so no filtering needed.
src/specify_cli/extensions.py
Outdated
| for script in manifest.scripts: | ||
| script_path = dest_dir / script["file"] | ||
| if script_path.exists() and script_path.suffix == ".sh": | ||
| script_path.chmod(script_path.stat().st_mode | 0o111) |
There was a problem hiding this comment.
chmod() on installed scripts can raise (e.g., restrictive FS permissions, read-only checkouts, or uncommon POSIX filesystems) and will currently abort the entire extension install. Consider making the executable-bit update best-effort (wrap in try/except OSError and continue) so a permission edge case doesn't prevent installation of an otherwise valid extension.
| script_path.chmod(script_path.stat().st_mode | 0o111) | |
| try: | |
| script_path.chmod(script_path.stat().st_mode | 0o111) | |
| except OSError: | |
| # Best-effort: ignore permission errors when setting executable bit | |
| pass |
There was a problem hiding this comment.
Fixed in aa1df03 — chmod is now wrapped in try/except OSError so permission errors don't abort the install.
- Make chmod best-effort (try/except OSError) so permission edge cases don't abort extension installation - Filter overrides by name pattern in list_available(): commands must contain dots, templates must not, preventing cross-contamination in the shared overrides directory Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Brings the extension system to parity with presets across three areas:
Extension system: Add template type diversity (scripts support) #1847 — Scripts support: Extensions now accept
provides.scriptsalongside commands in the manifest. Includes name format validation (^[a-z0-9-]+$), path safety checks (no traversal), executable permissions on.shfiles during install, and CLI display inextension list,extension add, andextension info.Extension system: Add extension filtering for ext-specific commands #1848 — Command filtering: Adds
_filter_commands_for_installed_extensions()toCommandRegistrar— filters extension-specific commands (speckit.<ext-id>.<cmd>) against installed extension directories. Mirrors the preset filtering logic atpresets.py:518-529. Available as a utility for cross-boundary filtering (e.g. when presets provide commands for extensions that may not be installed). Not applied during extension self-registration since all commands in an extension's own manifest are always valid.Extension system: Add template resolution system #1846 — Template resolution & discovery: Introduces
ExtensionResolver— a dedicated class inextensions.pythat owns extension template resolution, source attribution, and discovery.PresetResolvernow delegates its tier-3 (extension) lookups toExtensionResolverinstead of walking extension directories directly. Newspecify preset list-templates --type <type>CLI command for template discovery across the full 4-tier stack.Why ExtensionResolver instead of using PresetResolver?
The
PresetResolveralready had extension logic baked in (tier 3 of its resolution stack), but this meant extensions had to go through the preset system to discover their own templates — mixing concerns.ExtensionResolvergives extensions their own resolution/discovery API:resolve(name, type)— find a template from extensionsresolve_with_source(name, type)— with source attributionlist_templates(type)— discover all templates provided by extensionsPresetResolverremains the unified resolver across all 4 tiers (overrides → presets → extensions → core) but now delegates toExtensionResolverfor the extension tier rather than owning that logic directly. Each system owns its own domain.Closes #1846, closes #1847, closes #1848
Test plan
test_search_with_cached_dataunrelated to this PR)specify extension addwith a scripts-only extensionspecify preset list-templatesoutputs templates with correct source attribution🤖 Generated with Claude Code