Skip to content

feat(extensions): scripts support, command filtering, and template discovery#1964

Draft
mbachorik wants to merge 8 commits intogithub:mainfrom
mbachorik:feat/extension-parity
Draft

feat(extensions): scripts support, command filtering, and template discovery#1964
mbachorik wants to merge 8 commits intogithub:mainfrom
mbachorik:feat/extension-parity

Conversation

@mbachorik
Copy link
Copy Markdown
Contributor

@mbachorik mbachorik commented Mar 24, 2026

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.scripts alongside commands in the manifest. Includes name format validation (^[a-z0-9-]+$), path safety checks (no traversal), executable permissions on .sh files during install, and CLI display in extension list, extension add, and extension info.

  • Extension system: Add extension filtering for ext-specific commands #1848 — Command filtering: Adds _filter_commands_for_installed_extensions() to CommandRegistrar — filters extension-specific commands (speckit.<ext-id>.<cmd>) against installed extension directories. Mirrors the preset filtering logic at presets.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 in extensions.py that owns extension template resolution, source attribution, and discovery. PresetResolver now delegates its tier-3 (extension) lookups to ExtensionResolver instead of walking extension directories directly. New specify preset list-templates --type <type> CLI command for template discovery across the full 4-tier stack.

Why ExtensionResolver instead of using PresetResolver?

The PresetResolver already 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. ExtensionResolver gives extensions their own resolution/discovery API:

  • resolve(name, type) — find a template from extensions
  • resolve_with_source(name, type) — with source attribution
  • list_templates(type) — discover all templates provided by extensions

PresetResolver remains the unified resolver across all 4 tiers (overrides → presets → extensions → core) but now delegates to ExtensionResolver for the extension tier rather than owning that logic directly. Each system owns its own domain.

Closes #1846, closes #1847, closes #1848

Test plan

  • 843 tests pass (1 pre-existing failure in test_search_with_cached_data unrelated to this PR)
  • Verify specify extension add with a scripts-only extension
  • Verify specify preset list-templates outputs templates with correct source attribution
  • Verify command filtering works when a preset provides commands for an uninstalled extension

🤖 Generated with Claude Code

…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>
Copilot AI review requested due to automatic review settings March 24, 2026 21:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 .sh executable bits, and CLI surface area (list/add/info).
  • Introduce CommandRegistrar._filter_commands_for_installed_extensions() plus tests to filter speckit.<ext-id>.<cmd> commands when the target extension isn’t installed.
  • Add PresetResolver.list_available() and a new specify 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.

iamaeroplane and others added 2 commits March 25, 2026 09:11
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>
Copilot AI review requested due to automatic review settings March 25, 2026 12:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

iamaeroplane and others added 2 commits March 25, 2026 20:45
- 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>
After the ExtensionResolver refactor, PresetResolver no longer uses
ExtensionRegistry directly — that dependency moved into ExtensionResolver.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 26, 2026 15:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Coerce None values from provides.commands/scripts via `or []` and
  validate they are lists, preventing TypeError on null YAML values
- Discover both .sh and .ps1 scripts in ExtensionResolver and
  PresetResolver.list_available() instead of only .sh
- Remove unused ExtensionRegistry import (ruff F401)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/specify_cli/extensions.py:160

  • The commands/scripts validators assume each entry is a dict with string fields. If a manifest has provides.commands: ["foo"] or name/file values that aren’t strings, this will raise TypeError/KeyError (e.g., on cmd["name"] / regex) rather than a ValidationError. Validate isinstance(cmd, dict) and that name/file are strings before running regex/path checks.
        # Validate commands
        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 not re.match(r'^speckit\.[a-z0-9-]+\.[a-z0-9-]+$', cmd["name"]):

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add type checks for extension/requires/provides top-level fields
  (must be mappings, not null) to raise ValidationError instead of
  AttributeError on malformed manifests
- Add type checks for script entries (must be dict with string
  name/file) to prevent TypeError on non-dict or non-string values
- PresetResolver.resolve() now tries both .sh and .ps1 for scripts
  across all tiers, consistent with list_available() discovery

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

- Add type checks for command entries (must be dict with string
  name/file), mirroring the script validation added earlier
- ExtensionResolver._type_config raises ValueError for unsupported
  template types instead of silently falling back

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1707 to +1720
def _name_matches_type(name: str) -> bool:
"""Check if a file name matches the expected pattern for the template type.

Commands use dot notation (e.g. speckit.specify), templates use
hyphens only (e.g. spec-template). This prevents the shared
overrides directory from leaking commands into template listings
or vice versa. Scripts live in their own subdirectory so no
filtering is needed.
"""
if template_type == "command":
return "." in name
if template_type == "template":
return "." not in name
return True
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list_available()'s _name_matches_type() currently treats any .md filename without dots as a "template". That will cause non-template docs like README.md/CHANGELOG.md in preset/extension roots to show up in preset list-templates --type template. Consider validating discovered names against the same naming rules as manifests (e.g., ^[a-z0-9-]+$ for template/script, and a stricter speckit.* pattern for command) so discovery output only includes real templates.

Suggested change
def _name_matches_type(name: str) -> bool:
"""Check if a file name matches the expected pattern for the template type.
Commands use dot notation (e.g. speckit.specify), templates use
hyphens only (e.g. spec-template). This prevents the shared
overrides directory from leaking commands into template listings
or vice versa. Scripts live in their own subdirectory so no
filtering is needed.
"""
if template_type == "command":
return "." in name
if template_type == "template":
return "." not in name
return True
# Naming rules should match manifest expectations:
# - templates/scripts: lowercase letters, numbers, hyphens
# - commands: start with "speckit." and use dot notation
_TEMPLATE_SCRIPT_NAME_RE = re.compile(r"^[a-z0-9-]+$")
_COMMAND_NAME_RE = re.compile(r"^speckit\.[a-z0-9.]+$")
def _name_matches_type(name: str) -> bool:
"""Check if a file name matches the expected pattern for the template type.
Commands use dot notation (e.g. speckit.specify), templates use
hyphens only (e.g. spec-template). This prevents the shared
overrides directory from leaking commands into template listings
or vice versa. Scripts live in their own subdirectory so only
basic name validation is needed.
"""
if template_type == "command":
# Require "speckit.*" style names using dot notation.
return _COMMAND_NAME_RE.match(name) is not None
if template_type == "template":
# Require manifest-style names and prevent dot-notation leaks.
return (
"." not in name
and _TEMPLATE_SCRIPT_NAME_RE.match(name) is not None
)
# script: validate against the same naming rules as manifests
return _TEMPLATE_SCRIPT_NAME_RE.match(name) is not None

Copilot uses AI. Check for mistakes.
Comment on lines +1726 to +1738
for f in sorted(directory.iterdir()):
if f.is_file() and f.suffix in exts:
name = f.stem
if name in seen:
continue
if not _name_matches_type(name):
continue
seen.add(name)
results.append({
"name": name,
"path": str(f),
"source": source,
})
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For template_type == "script", _collect() deduplicates by stem using the filesystem sort order. If both name.sh and name.ps1 exist, the entry chosen here can differ from resolve() (which tries .sh before .ps1). Consider making discovery choose the same preferred extension order as resolution (e.g., pick .sh first when both exist) to keep list_available() consistent with what will actually resolve.

Suggested change
for f in sorted(directory.iterdir()):
if f.is_file() and f.suffix in exts:
name = f.stem
if name in seen:
continue
if not _name_matches_type(name):
continue
seen.add(name)
results.append({
"name": name,
"path": str(f),
"source": source,
})
# For scripts, multiple files with the same stem (e.g. name.sh and
# name.ps1) may exist in a single directory. We want discovery to
# choose the same preferred extension order as resolution, which
# is defined by the order of `exts` (".sh" before ".ps1").
if template_type == "script":
candidates: dict[str, Path] = {}
for f in sorted(directory.iterdir()):
if f.is_file() and f.suffix in exts:
name = f.stem
if not _name_matches_type(name):
continue
existing = candidates.get(name)
if existing is None:
candidates[name] = f
else:
# Prefer the extension that appears earlier in `exts`.
if exts.index(f.suffix) < exts.index(existing.suffix):
candidates[name] = f
for name, f in sorted(candidates.items(), key=lambda item: item[0]):
if name in seen:
continue
seen.add(name)
results.append({
"name": name,
"path": str(f),
"source": source,
})
else:
for f in sorted(directory.iterdir()):
if f.is_file() and f.suffix in exts:
name = f.stem
if name in seen:
continue
if not _name_matches_type(name):
continue
seen.add(name)
results.append({
"name": name,
"path": str(f),
"source": source,
})

Copilot uses AI. Check for mistakes.
Comment on lines +1023 to +1032
for f in sorted(scan_dir.iterdir()):
if f.is_file() and f.suffix in exts:
name = f.stem
if name not in seen:
seen.add(name)
results.append({
"name": name,
"path": str(f),
"source": source_label,
})
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExtensionResolver.list_templates() currently accepts any matching file and uses f.stem as the template name without validating it for the requested template_type. In practice this can surface invalid command names (e.g. commands/hello.md becomes hello) and can also pick .ps1 over .sh purely due to filename sort order when both exist (which may disagree with resolve()'s extension precedence). Consider adding the same per-type name filtering as PresetResolver.list_available() and ensuring script deduplication follows the same suffix preference as resolve().

Copilot uses AI. Check for mistakes.
Comment on lines 121 to 143
@@ -135,18 +137,34 @@ def _validate(self):

# Validate requires section
requires = self.data["requires"]
if not isinstance(requires, dict):
raise ValidationError("'requires' must be a mapping")
if "speckit_version" not in requires:
raise ValidationError("Missing requires.speckit_version")
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExtensionManifest._validate() assumes extension.id/extension.version (and requires.speckit_version) are strings. If YAML provides non-strings (e.g. id: 123, version: 1.0), re.match(...), pkg_version.Version(...), or SpecifierSet(...) can raise TypeError and bypass the intended ValidationError path. Consider adding explicit type checks for these required metadata fields (and requires.speckit_version) so malformed manifests fail with a consistent ValidationError instead of crashing.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants