Skip to content

Commit 684b3d8

Browse files
authored
feat(extensions): accept provides.templates and provides.scripts in manifest (github#4012)
* feat(extensions): accept provides.templates and provides.scripts in manifest Extensions could only formally declare commands under `provides` (plus config/hooks/events); templates and scripts shipped by an extension were picked up purely by filename convention, with no id, description, or metadata. Add optional `provides.templates` and `provides.scripts` sections to the extension manifest schema, mirroring the preset template shape minus an authorable `strategy` (extension artifacts always resolve as replace, so a present `strategy` key is now a validation error rather than a silently accepted no-op). ExtensionManifest gains `templates`/`scripts` properties so tooling can enumerate an extension's declared artifacts directly from the manifest. An extension may now satisfy the "must provide something" rule with only a template or script, not just a command/hook/event. Addresses the manifest-schema portion of github#4010; resolver authoritative-vs-convention precedence for these new sections is left for a follow-up. * fix(presets): wire extension-declared templates/scripts into resolver collect_all_layers only consulted ExtensionManifest for command resolution, leaving provides.templates/.scripts purely decorative -- a declared entry whose file didn't sit at the conventional path was validated but never resolved. Extend the existing manifest-fallback branch to cover template_type "template" and "script" the same way it already does "command": convention lookup first, manifest lookup as fallback so undeclared on-disk files keep resolving unchanged. * fix(presets): make extension manifest lookup authoritative over convention Copilot review on github#4012 found the manifest-declared template/script lookup was gated on convention lookup missing first, so a stale conventional file could shadow a declared entry at a non-conventional path, and resolve() never consulted the manifest at all (only collect_all_layers() did). Add a shared _extension_manifest_declared_template() helper and check it before convention-based lookup in both resolve() and collect_all_layers(), mirroring the preset manifest precedence. Also update EXTENSION-DEVELOPMENT-GUIDE.md, which still claimed provides only supports commands and required a command or hook. * fix(presets): stop resolving symlinks in extension manifest candidate path _extension_manifest_declared_template() resolved ext_dir/rel_path before returning it, which follows symlinks in ext_dir's ancestors (e.g. macOS's symlinked tmp dir) and diverges from the unresolved paths convention-based lookup returns for the same directory. Resolve only for the traversal containment check; return the unresolved candidate. Fixes the 4 CI test failures across all OS/Python matrix jobs on github#4012.
1 parent 247abbf commit 684b3d8

6 files changed

Lines changed: 646 additions & 26 deletions

File tree

extensions/EXTENSION-API-REFERENCE.md

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,25 @@ requires:
4040
required: boolean # Optional, default: false
4141

4242
provides:
43-
commands: # Required, at least one command
43+
commands: # At least one of commands/templates/scripts/hooks/events required
4444
- name: string # Required, pattern: ^speckit\.[a-z0-9-]+\.[a-z0-9-]+$
4545
file: string # Required, relative path to command file
4646
description: string # Required
4747
aliases: [string] # Optional, same pattern as name; namespace must match extension.id and must not shadow core or installed extension commands
4848

49+
templates: # Optional, array of declared templates. Always resolve
50+
# as "replace" -- 'strategy' is not an authorable field here.
51+
- name: string # Required, pattern: ^[a-z0-9-]+$
52+
file: string # Required, relative path to template file
53+
description: string # Optional
54+
55+
scripts: # Optional, array of declared scripts. Always resolve
56+
# as "replace" -- 'strategy' is not an authorable field here.
57+
- name: string # Required, pattern: ^[a-z0-9-]+$
58+
file: string # Required, relative path to script file
59+
description: string # Optional
60+
runtimes: [string] # Optional, subset of: bash, powershell, python
61+
4962
config: # Optional, array of config files
5063
- name: string # Config file name
5164
template: string # Template file path
@@ -111,6 +124,29 @@ defaults: # Optional, default configuration values
111124
- **Examples**: `speckit.jira.specstoissues`, `speckit.linear.sync`
112125
- **Invalid**: `jira.specstoissues`, `speckit.command`, `speckit.jira.CreateIssues`
113126

127+
#### `provides.templates[].name` / `provides.scripts[].name`
128+
129+
- **Type**: string
130+
- **Pattern**: `^[a-z0-9-]+$`
131+
- **Description**: Unlike commands, templates and scripts are not invoked by
132+
name, so they use the same plain slug pattern as `extension.id` rather than
133+
the namespaced command pattern.
134+
- **Examples**: `myext-template`, `myext-collect`
135+
136+
#### `provides.templates[].strategy` / `provides.scripts[].strategy`
137+
138+
- Not an authorable field. Extension-contributed templates and scripts are
139+
always resolved as `replace`; a manifest that includes a `strategy` key on
140+
one of these entries is rejected with a `ValidationError`. Composable
141+
strategies (`wrap`/`prepend`/`append`) are preset-only.
142+
143+
#### `provides.scripts[].runtimes`
144+
145+
- **Type**: array of strings
146+
- **Values**: `bash`, `powershell`, `python`
147+
- **Description**: Declares which runtimes the script supports. Purely
148+
informational metadata — it is not used to select or invoke the script.
149+
114150
#### `hooks`
115151

116152
- **Type**: object
@@ -143,6 +179,8 @@ manifest.version # str: Version
143179
manifest.description # str: Description
144180
manifest.requires_speckit_version # str: Required spec-kit version
145181
manifest.commands # List[Dict]: Command definitions
182+
manifest.templates # List[Dict]: Declared template definitions
183+
manifest.scripts # List[Dict]: Declared script definitions
146184
manifest.hooks # Dict: Hook definitions
147185
```
148186

extensions/EXTENSION-DEVELOPMENT-GUIDE.md

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,11 @@ Compatibility requirements.
177177

178178
What the extension provides.
179179

180-
**Optional sub-fields**:
180+
**Optional sub-fields** (at least one of `commands`, `templates`, `scripts`, `hooks`, or `events` is required):
181181

182-
- `commands`: Array of command objects (at least one command or hook is required)
182+
- `commands`: Array of command objects
183+
- `templates`: Array of template objects
184+
- `scripts`: Array of script objects
183185

184186
**Command object**:
185187

@@ -188,6 +190,21 @@ What the extension provides.
188190
- `description`: Command description (optional)
189191
- `aliases`: Alternative command names (optional, array; each must match `speckit.{ext-id}.{command}`)
190192

193+
**Template object**:
194+
195+
- `name`: Template name (lowercase, alphanumeric, hyphens — e.g. `myext-template`)
196+
- `file`: Path to template file (relative to extension root)
197+
- `description`: Template description (optional)
198+
199+
**Script object**:
200+
201+
- `name`: Script name (lowercase, alphanumeric, hyphens — e.g. `myext-collect`)
202+
- `file`: Path to script file (relative to extension root)
203+
- `description`: Script description (optional)
204+
- `runtimes`: Runtimes the script supports (optional, array; subset of `bash`, `powershell`, `python` — informational only, not used to select or invoke the script)
205+
206+
Extension-provided templates and scripts always resolve as `replace`; a manifest that includes a `strategy` key on one of these entries is rejected with a `ValidationError`. Composable strategies (`wrap`/`prepend`/`append`) are preset-only.
207+
191208
### Optional Fields
192209

193210
#### `hooks`

src/specify_cli/extensions/__init__.py

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@
6161
)
6262
EXTENSION_COMMAND_NAME_PATTERN = re.compile(r"^speckit\.([a-z0-9-]+)\.([a-z0-9-]+)$")
6363

64+
# Naming pattern for provides.templates / provides.scripts entries. Unlike
65+
# commands, these are not namespaced (they aren't invoked via a command
66+
# name), so they follow the same plain slug pattern as extension.id.
67+
VALID_EXTENSION_ARTIFACT_NAME_PATTERN = re.compile(r"^[a-z0-9-]+$")
68+
69+
VALID_SCRIPT_RUNTIMES = frozenset({"bash", "powershell", "python"})
70+
6471
VALID_EFFECTS = frozenset({"read-only", "read-write"})
6572

6673
DEFAULT_HOOK_PRIORITY = 10
@@ -368,11 +375,17 @@ def _validate(self):
368375
f"Invalid provides: expected a mapping, got {type(provides).__name__}"
369376
)
370377
commands = provides.get("commands", [])
378+
templates = provides.get("templates", [])
379+
scripts = provides.get("scripts", [])
371380
hooks = self.data.get("hooks")
372381
events = self.data.get("events")
373382

374383
if "commands" in provides and not isinstance(commands, list):
375384
raise ValidationError("Invalid provides.commands: expected a list")
385+
if "templates" in provides and not isinstance(templates, list):
386+
raise ValidationError("Invalid provides.templates: expected a list")
387+
if "scripts" in provides and not isinstance(scripts, list):
388+
raise ValidationError("Invalid provides.scripts: expected a list")
376389
if "hooks" in self.data and not isinstance(hooks, dict):
377390
raise ValidationError("Invalid hooks: expected a mapping")
378391
if "events" in self.data:
@@ -382,9 +395,17 @@ def _validate(self):
382395
has_commands = bool(commands)
383396
has_hooks = bool(hooks)
384397
has_events = bool(events)
398+
has_templates = bool(templates)
399+
has_scripts = bool(scripts)
400+
401+
if not has_commands and not has_hooks and not has_events and not has_templates and not has_scripts:
402+
raise ValidationError(
403+
"Extension must provide at least one command, hook, or event "
404+
"(or a declared template/script)"
405+
)
385406

386-
if not has_commands and not has_hooks and not has_events:
387-
raise ValidationError("Extension must provide at least one command, hook, or event")
407+
self._validate_provided_artifacts(templates, section="templates", singular="template")
408+
self._validate_provided_artifacts(scripts, section="scripts", singular="script")
388409

389410
# Validate hook values (if present).
390411
# Each event is a single mapping or a list of mappings.
@@ -545,6 +566,70 @@ def _validate(self):
545566
f"The extension author should update the manifest."
546567
)
547568

569+
@staticmethod
570+
def _validate_provided_artifacts(entries: List[Any], section: str, singular: str) -> None:
571+
"""Validate provides.templates / provides.scripts entries.
572+
573+
Mirrors the shape/path-safety checks PresetManifest applies to its
574+
non-command templates, minus 'type' (the section name already
575+
distinguishes template vs script) and 'strategy' (extension-provided
576+
artifacts are always 'replace' -- see the forced-replace resolver
577+
behavior for extension layers in presets/__init__.py). A present
578+
'strategy' key is rejected rather than silently ignored, so an author
579+
who copies a preset-style entry gets a clear error instead of a
580+
silently-dropped field.
581+
"""
582+
for entry in entries:
583+
if not isinstance(entry, dict):
584+
raise ValidationError(
585+
f"Each entry in 'provides.{section}' must be a mapping"
586+
)
587+
if "name" not in entry or "file" not in entry:
588+
raise ValidationError(f"{singular.capitalize()} missing 'name' or 'file'")
589+
590+
name = entry["name"]
591+
if not isinstance(name, str):
592+
raise ValidationError(
593+
f"Invalid {singular} name: expected a string, got {type(name).__name__}"
594+
)
595+
if not VALID_EXTENSION_ARTIFACT_NAME_PATTERN.match(name):
596+
raise ValidationError(
597+
f"Invalid {singular} name '{name}': "
598+
"must be lowercase alphanumeric with hyphens only"
599+
)
600+
601+
file_value = entry["file"]
602+
reason = relative_extension_path_violation(file_value)
603+
if reason:
604+
label = repr(file_value) if isinstance(file_value, str) else f"for {singular} '{name}'"
605+
raise ValidationError(f"Invalid {singular} 'file' {label}: {reason}")
606+
607+
if "description" in entry and not isinstance(entry["description"], str):
608+
raise ValidationError(
609+
f"Invalid {singular} description for '{name}': expected a string"
610+
)
611+
612+
if "strategy" in entry:
613+
raise ValidationError(
614+
f"Invalid {singular} entry '{name}': 'strategy' is not authorable for "
615+
"extension-provided artifacts, which always use 'replace' semantics"
616+
)
617+
618+
if section == "scripts" and "runtimes" in entry:
619+
runtimes = entry["runtimes"]
620+
if not isinstance(runtimes, list) or not all(
621+
isinstance(r, str) for r in runtimes
622+
):
623+
raise ValidationError(
624+
f"Invalid runtimes for script '{name}': expected a list of strings"
625+
)
626+
invalid = sorted(set(runtimes) - VALID_SCRIPT_RUNTIMES)
627+
if invalid:
628+
raise ValidationError(
629+
f"Invalid runtimes {invalid} for script '{name}': "
630+
f"must be one of {sorted(VALID_SCRIPT_RUNTIMES)}"
631+
)
632+
548633
@staticmethod
549634
def _try_correct_command_name(name: str, ext_id: str) -> Optional[str]:
550635
"""Try to auto-correct a non-conforming command name to the required pattern.
@@ -615,6 +700,16 @@ def config(self) -> List[Dict[str, Any]]:
615700
return []
616701
return raw
617702

703+
@property
704+
def templates(self) -> List[Dict[str, Any]]:
705+
"""Get list of declared templates (provides.templates)."""
706+
return self.data.get("provides", {}).get("templates", [])
707+
708+
@property
709+
def scripts(self) -> List[Dict[str, Any]]:
710+
"""Get list of declared scripts (provides.scripts)."""
711+
return self.data.get("provides", {}).get("scripts", [])
712+
618713
@property
619714
def hooks(self) -> Dict[str, Any]:
620715
"""Get hook definitions."""

src/specify_cli/presets/__init__.py

Lines changed: 77 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4995,6 +4995,64 @@ def _manifest_declared_template(
49954995
return tmpl, None
49964996
return None, None
49974997

4998+
def _extension_manifest_declared_template(
4999+
self, ext_dir: Path, template_name: str, template_type: str
5000+
) -> tuple[dict | None, Path | None]:
5001+
"""Resolve an extension's manifest-declared command/template/script entry and usable file.
5002+
5003+
Mirrors ``_manifest_declared_template`` (for presets): returns ``(entry, candidate)``
5004+
where ``entry`` is the matching ``provides.<type>`` mapping, or ``None`` if the
5005+
extension has no (valid) manifest or doesn't declare this ``(name, type)``.
5006+
``candidate`` is the declared ``file:`` resolved under ``ext_dir`` IFF it is a
5007+
regular file that stays within ``ext_dir`` (guards against path traversal via a
5008+
malformed manifest, mirroring ``resolve_extension_command_via_manifest``);
5009+
``None`` otherwise.
5010+
5011+
The manifest is authoritative: when ``entry`` is not ``None`` but ``candidate`` is
5012+
``None``, callers must NOT fall back to convention-based lookup — that would mask
5013+
a typo or pick up an undeclared file. Shared by ``resolve()`` and
5014+
``collect_all_layers()`` so their manifest-first resolution cannot silently
5015+
diverge (the divergence flagged in review on #4012).
5016+
"""
5017+
if template_type not in ("command", "template", "script"):
5018+
return None, None
5019+
ext_manifest_path = ext_dir / "extension.yml"
5020+
if not ext_manifest_path.exists():
5021+
return None, None
5022+
from ..extensions import ExtensionManifest, ValidationError as ExtValidationError
5023+
5024+
try:
5025+
ext_manifest = ExtensionManifest(ext_manifest_path)
5026+
except (ExtValidationError, yaml.YAMLError, OSError, TypeError, AttributeError):
5027+
return None, None
5028+
if template_type == "command":
5029+
entries = ext_manifest.commands
5030+
elif template_type == "template":
5031+
entries = ext_manifest.templates
5032+
else:
5033+
entries = ext_manifest.scripts
5034+
for entry in entries:
5035+
if entry.get("name") != template_name:
5036+
continue
5037+
file_rel = entry.get("file")
5038+
if not file_rel:
5039+
return entry, None
5040+
rel_path = Path(file_rel)
5041+
if rel_path.is_absolute():
5042+
return entry, None
5043+
candidate = ext_dir / rel_path
5044+
try:
5045+
# Resolve only for the containment check, not for the
5046+
# returned path -- resolving the returned path would follow
5047+
# symlinks in ext_dir's ancestors (e.g. a symlinked tmp dir
5048+
# on macOS) and diverge from the unresolved paths convention
5049+
# lookup returns for the same directory.
5050+
candidate.resolve().relative_to(ext_dir.resolve()) # raises ValueError if outside
5051+
except (OSError, ValueError):
5052+
return entry, None
5053+
return entry, (candidate if candidate.is_file() else None)
5054+
return None, None
5055+
49985056
def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]:
49995057
"""Build unified list of registered and unregistered extensions sorted by priority.
50005058
@@ -5131,6 +5189,16 @@ def resolve(
51315189
ext_dir = self.extensions_dir / ext_id
51325190
if not ext_dir.is_dir():
51335191
continue
5192+
# The extension manifest is authoritative, same as preset manifests
5193+
# above: check it before convention-based lookup so a declared entry
5194+
# at a non-conventional path wins over a stale conventional file.
5195+
entry, manifest_candidate = self._extension_manifest_declared_template(
5196+
ext_dir, template_name, template_type
5197+
)
5198+
if manifest_candidate is not None:
5199+
return manifest_candidate
5200+
if entry is not None:
5201+
continue
51345202
for subdir in subdirs:
51355203
if subdir:
51365204
candidate = ext_dir / subdir / f"{template_name}{ext}"
@@ -5440,27 +5508,15 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]:
54405508
ext_dir = self.extensions_dir / ext_id
54415509
if not ext_dir.is_dir():
54425510
continue
5443-
# Try convention-based lookup first
5444-
candidate = _find_in_subdirs(ext_dir)
5445-
# If not found and this is a command, check extension manifest
5446-
if candidate is None and template_type == "command":
5447-
ext_manifest_path = ext_dir / "extension.yml"
5448-
if ext_manifest_path.exists():
5449-
try:
5450-
from ..extensions import ExtensionManifest, ValidationError as ExtValidationError
5451-
ext_manifest = ExtensionManifest(ext_manifest_path)
5452-
for cmd in ext_manifest.commands:
5453-
if cmd.get("name") == template_name:
5454-
cmd_file = cmd.get("file")
5455-
if cmd_file:
5456-
c = ext_dir / cmd_file
5457-
if c.exists():
5458-
candidate = c
5459-
break
5460-
except (ExtValidationError, yaml.YAMLError):
5461-
# Invalid extension manifest — fall back to
5462-
# convention-based lookup (already attempted above).
5463-
pass
5511+
# The extension manifest is authoritative, same as preset manifests
5512+
# above: check it before convention-based lookup so a declared entry
5513+
# at a non-conventional path wins over a stale conventional file, and
5514+
# a declared-but-missing file isn't silently masked by convention.
5515+
entry, candidate = self._extension_manifest_declared_template(
5516+
ext_dir, template_name, template_type
5517+
)
5518+
if entry is None:
5519+
candidate = _find_in_subdirs(ext_dir)
54645520
if candidate:
54655521
if ext_meta:
54665522
version = ext_meta.get("version", "?")

0 commit comments

Comments
 (0)