diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 1098abfb42..5093a424ff 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -205,6 +205,35 @@ specify preset add team-workflow --priority 10 For any file that both provide, `compliance` wins (priority 5 < 10). For files only one provides, that one is used. For files neither provides, the core default is used. +## Contribution Identifiers + +Every command, template, and script contributed by a preset (or an extension, or the core layer) is addressable at read time by a deterministic opaque identifier of the form: + +```text +{layer}:{sourceId}:{kind}:{name} +``` + +- `layer` is one of `core`, `preset`, or `extension`. +- `sourceId` is `_` for `core`, the preset pack id for `preset`, or the extension id for `extension`. +- `kind` is one of `command`, `template`, or `script` (see below for hooks). +- `name` is the entry's declared `name` field. + +`sourceId` is the source's own stable system identifier: + +| Layer | `sourceId` value | Where it comes from | +| --- | --- | --- | +| `core` | `_` (literal underscore) | Placeholder because core has no manifest id | +| `preset` | The preset pack's `id` | The manifest's `preset.id` field — the same value used by `PresetManifest.id`, the install directory, registries, and resolver layer metadata | +| `extension` | The extension's `id` | The manifest's `extension.id` field — the value used by `ExtensionManifest.id` and manifest-backed `lookupId` values; the extension directory or registry key may differ for unregistered local copies | + +Preset contribution identifiers cover named artifacts (`command`, `template`, and `script`). Extension hooks are treated separately because their lookup name is derived from the hook event and command instead of a single `name` field. + +Identifiers are computed on demand from author-declared manifest content and are never persisted to `.specify/` or any cache. Copying a preset to another machine (or touching its files) does not change the identifiers it produces. + +`PresetResolver.collect_all_layers()` returns layer dicts that each include a `lookupId` field pointing back to the originating contribution's `id`. Project-local overrides in `.specify/templates/overrides/` are a resolver-only concept — they carry a synthetic `project:_:{kind}:{name}` `lookupId` that intentionally does not match any manifest contribution. + +For the full grammar, including the hook name-component convention and extension hook duplicate collapse semantics, see the [Extension API Reference — Contribution Identifiers](../../extensions/EXTENSION-API-REFERENCE.md#contribution-identifiers) section. + ## FAQ ### Can I use multiple presets at the same time? diff --git a/extensions/EXTENSION-API-REFERENCE.md b/extensions/EXTENSION-API-REFERENCE.md index a7bece0b89..66617d0e9c 100644 --- a/extensions/EXTENSION-API-REFERENCE.md +++ b/extensions/EXTENSION-API-REFERENCE.md @@ -10,6 +10,7 @@ Technical reference for Spec Kit extension system APIs and manifest schema. 4. [Configuration Schema](#configuration-schema) 5. [Hook System](#hook-system) 6. [CLI Commands](#cli-commands) +7. [Contribution Identifiers](#contribution-identifiers) --- @@ -859,7 +860,65 @@ satisfied = version_satisfies("1.2.3", ">=1.0.0,<2.0.0") # bool --- -## File System Layout +## Contribution Identifiers + +Every command, template, script, and hook contributed by an extension (or a preset, or the core layer) is addressable at read time by a deterministic opaque identifier. Resolved artifact-stack layers carry a matching `lookupId` field that points back to the contribution the layer came from. Identifiers are **computed on demand from author-declared manifest content** and are **never persisted** to `.specify/` or to any cache file. + +### Grammar + +Named contributions (commands, templates, scripts) follow: + +```text +{layer}:{sourceId}:{kind}:{name} +``` + +- `layer` is one of `core`, `preset`, or `extension`. +- `sourceId` is `_` for `core`, the preset pack id for `preset`, or the extension id for `extension`. +- `kind` is one of `command`, `template`, `script`, or `hook`. +- `name` is the contribution's declared `name` field. + +`sourceId` is the source's own stable manifest identifier: `_` for the manifest-less core layer, `preset.id` from `preset.yml` for presets, and `extension.id` from `extension.yml` for extensions. Manifest-backed extension contributions and resolver `lookupId` values use `extension.id` even when an unregistered local copy has a different directory or registry key, which makes identifiers stable join keys back to their originating manifest. + +Hook contributions use a compound name-component built from the event and command: + +```text +{layer}:{sourceId}:hook:{eventName}:{command} +``` + +If an extension declares multiple hooks with the same `(eventName, command)` pair, the final declaration wins, matching hook installation. `iter_contributions()` emits only that final hook, so every emitted hook identifier corresponds to an installed hook. + +### Reserved character + +`:` is reserved as the identifier component separator. It cannot appear inside any of `layer`, `sourceId`, `kind`, `name`, `eventName`, or `command`. Extension ids, command names, template names, and script names are already constrained by their existing regex patterns (`^[a-z0-9-]+$` and friends), which forbid `:`. In addition, every value that appears in an identifier — contribution names (commands, templates, scripts) as well as hook event names (mapping keys) and hook `command` values — is explicitly validated to reject `:` at manifest load, so the guarantee holds uniformly. + +### The `project:` sentinel + +Project-local overrides in `.specify/templates/overrides/` are a resolver-only concept — they have no backing manifest and cannot appear in `iter_contributions()`. Layers of that kind carry a synthetic `lookupId` of the form `project:_:{kind}:{name}` so consumers that reverse-lookup the id always see "not found", which is the intended behaviour: overrides are addressable at the stack level, not as first-class contributions. + +### Python API + +```python +class ExtensionManifest: + def iter_contributions(self) -> list[dict]: ... + def contribution_id(self, kind: str, name: str) -> str | None: ... + + +class PresetManifest: + def iter_contributions(self) -> list[dict]: ... + def contribution_id(self, kind: str, name: str) -> str | None: ... +``` + +Each contribution dict carries `{layer, sourceId, kind, name, id, ...author-declared fields}`; `id` is the computed identifier. `contribution_id(kind, name)` returns the id for a single lookup, or `None` if no contribution matches. + +`PresetResolver.collect_all_layers()` returns layer dicts that include a `lookupId` field for every layer type (`project override`, preset, extension, core, and bundled core). + +### Determinism guarantees + +Identifier derivation reads only the in-memory declared manifest content. No filesystem paths, no `os.environ`, no timestamps, and no file-content hashes contribute to any id. Copying an extension or preset to a different machine (or renaming its directory, or touching its files) does not change the identifiers it produces. + +### Opacity guidance + +Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers. ```text .specify/ diff --git a/src/specify_cli/_identifier.py b/src/specify_cli/_identifier.py new file mode 100644 index 0000000000..2bd0540453 --- /dev/null +++ b/src/specify_cli/_identifier.py @@ -0,0 +1,99 @@ +"""Deterministic identifiers for Spec Kit contributions and resolved stack layers. + +Every command, template, script, and hook contribution surfaced by a preset or +extension manifest carries a computed opaque ``id`` string, and every layer of a +resolved artifact stack carries a matching ``lookupId``. The identifier value is +derived only from author-declared manifest data — it never depends on file +contents, timestamps, archive hashes, installation directory paths, install-time +random values, or list positions. That is what makes identifiers portable +across machines, project locations, and reinstalls, and what lets consumers use +them as stable join keys. + +Grammar for named contributions (commands, templates, scripts):: + + id = "{layer}:{sourceId}:{kind}:{name}" + + layer ∈ {"core", "preset", "extension"} + sourceId = "_" when layer == "core"; the preset id or extension id otherwise + kind ∈ {"command", "template", "script", "hook"} + name = the contribution's declared ``name`` + +Hook identifiers use ``{eventName}:{command}`` as the name component:: + + id = "{layer}:{sourceId}:hook:{eventName}:{command}" + +Extension hook contributions use the same last-write-wins behavior as hook +installation, so at most one contribution exists for each +``(eventName, command)`` pair. + +The functions in this module are pure — inputs are strings or in-memory +mappings parsed from a manifest, outputs are strings. None of them read from +disk, look at ``os.environ``, call ``datetime``, or hash file contents. That +guarantee is what preserves portability, and it is enforced by inspection +rather than by runtime checks: any change here that adds an ambient input is a +change that breaks the identifier contract. +""" + +from __future__ import annotations + +from typing import Any + + +PROJECT_OVERRIDE_LAYER = "project" +"""Resolver-only layer label for project-local override layers. + +Project overrides are a resolver feature — they are not backed by any manifest +contribution. When a resolved artifact stack contains a project-override layer, +its ``lookupId`` uses this label so the round-trip invariant (every layer +carries a ``lookupId``) still holds. No manifest ``iter_contributions()`` will +ever emit a matching ``id``, so consumers see "not found" for the lookup, which +is the correct outcome for a layer with no originating manifest entry. +""" + +class IdentifierComponentError(ValueError): + """Raised when a manifest component would break identifier grammar.""" + + +def validate_component(value: Any, field_label: str) -> str: + """Return ``value`` unchanged if it is a non-empty ``:``-free string. + + Manifest components that appear in an identifier (``layer``, ``sourceId``, + ``kind``, ``name``, ``eventName``, ``command``) may not contain the ``:`` + delimiter — the grammar has no escape rule. This function is the guard used + by manifest validators to reject offending values at load time with a clear + message naming the field. + """ + if not isinstance(value, str): + raise IdentifierComponentError( + f"Invalid {field_label}: expected a string, got {type(value).__name__}" + ) + if not value: + raise IdentifierComponentError( + f"Invalid {field_label}: value must not be empty" + ) + if ":" in value: + raise IdentifierComponentError( + f"Invalid {field_label} '{value}': ':' is reserved as an identifier delimiter" + ) + return value + + +def derive_named_id(layer: str, source_id: str, kind: str, name: str) -> str: + """Build the identifier string for a named contribution kind. + + Callers are expected to have already validated each component with + :func:`validate_component` at manifest-load time; this function does not + revalidate — it is a pure string join so the identifier can be computed + cheaply on every read. + """ + return f"{layer}:{source_id}:{kind}:{name}" + + +def derive_hook_id( + layer: str, + source_id: str, + event_name: str, + command: str, +) -> str: + """Build the identifier string for a hook contribution.""" + return f"{layer}:{source_id}:hook:{event_name}:{command}" diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 3968e4fcbe..34a27bef7c 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -28,6 +28,12 @@ from packaging.specifiers import InvalidSpecifier, SpecifierSet from .._assets import _locate_core_pack, _repo_root +from .._identifier import ( + IdentifierComponentError, + derive_hook_id, + derive_named_id, + validate_component, +) from .._download_security import ( archive_format_from_name, archive_suffix, @@ -213,6 +219,92 @@ def coerce_hook_entries(hook_config: Any) -> List[Any]: return hook_config if isinstance(hook_config, list) else [hook_config] +def collapse_hook_event_entries(event_name: str, hook_config: Any) -> List[Dict[str, Any]]: + """Collapse a hook event to final entries using installer last-write-wins. + + Duplicate commands are removed and re-inserted so the final declaration is + retained at the end of the event list, matching register-time ordering. + """ + collapsed: Dict[str, Dict[str, Any]] = {} + for entry in coerce_hook_entries(hook_config): + if not isinstance(entry, dict): + continue + command = entry.get("command") + if not command: + continue + normalized = dict(entry) + normalized.setdefault("eventName", event_name) + if command in collapsed: + del collapsed[command] + collapsed[command] = normalized + return list(collapsed.values()) + + +def _collect_extension_command_names( + extension_id: str, commands: List[Dict[str, Any]] +) -> Dict[str, str]: + """Collect and validate command and alias names declared by a manifest.""" + if extension_id in CORE_COMMAND_NAMES: + raise ValidationError( + f"Extension ID '{extension_id}' conflicts with core command namespace '{extension_id}'" + ) + + declared_names: Dict[str, str] = {} + + for cmd in commands: + primary_name = cmd["name"] + aliases = cmd.get("aliases", []) + + if aliases is None: + aliases = [] + if not isinstance(aliases, list): + raise ValidationError( + f"Aliases for command '{primary_name}' must be a list" + ) + + for kind, name in [("command", primary_name)] + [ + ("alias", alias) for alias in aliases + ]: + if not isinstance(name, str): + raise ValidationError( + f"{kind.capitalize()} for command '{primary_name}' must be a string" + ) + + path_reason = relative_extension_path_violation(name) + if path_reason: + raise ValidationError(f"Invalid {kind} {name!r}: {path_reason}") + + # Enforce canonical pattern only for primary command names; + # aliases are free-form to preserve community extension compat. + if kind == "command": + match = EXTENSION_COMMAND_NAME_PATTERN.match(name) + if match is None: + raise ValidationError( + f"Invalid {kind} '{name}': " + "must follow pattern 'speckit.{extension}.{command}'" + ) + + namespace = match.group(1) + if namespace != extension_id: + raise ValidationError( + f"{kind.capitalize()} '{name}' must use extension namespace '{extension_id}'" + ) + + if namespace in CORE_COMMAND_NAMES: + raise ValidationError( + f"{kind.capitalize()} '{name}' conflicts with core command namespace '{namespace}'" + ) + + if name in declared_names: + raise ValidationError( + f"Duplicate command or alias '{name}' in extension manifest" + ) + + declared_names[name] = kind + + return declared_names + + @dataclass class CatalogEntry(BaseCatalogEntry): """Represents a single catalog entry in the catalog stack.""" @@ -415,6 +507,11 @@ def _validate(self): raise ValidationError( f"Invalid hook '{hook_name}': list must contain at least one entry" ) + try: + validate_component(hook_name, f"hook event name '{hook_name}'") + except IdentifierComponentError as exc: + raise ValidationError(str(exc)) from exc + event_entries: List[dict] = [] for entry in coerce_hook_entries(hook_config): if not isinstance(entry, dict): raise ValidationError( @@ -425,6 +522,13 @@ def _validate(self): raise ValidationError( f"Hook '{hook_name}' missing required 'command' field" ) + try: + validate_component( + entry["command"], + f"hook '{hook_name}' command", + ) + except IdentifierComponentError as exc: + raise ValidationError(str(exc)) from exc if "priority" in entry: priority = entry["priority"] if not isinstance(priority, int) or isinstance(priority, bool): @@ -437,6 +541,7 @@ def _validate(self): f"Hook '{hook_name}' has invalid 'priority': " "must be >= 1" ) + event_entries.append(entry) # Validate commands; track renames so hook references can be rewritten. rename_map: Dict[str, str] = {} @@ -487,6 +592,15 @@ def _validate(self): "must follow pattern 'speckit.{extension}.{command}'" ) + # The (possibly corrected) name is an identifier component, so it + # may not contain the ':' delimiter. EXTENSION_COMMAND_NAME_PATTERN + # already excludes it; the explicit guard keeps the guarantee + # uniform with the hook fields. + try: + validate_component(cmd["name"], f"command name '{cmd['name']}'") + except IdentifierComponentError as exc: + raise ValidationError(str(exc)) from exc + # Validate alias types; no pattern enforcement on aliases — they are # intentionally free-form to preserve community extension compatibility # (e.g. 'speckit.verify' short aliases used by existing extensions). @@ -510,6 +624,8 @@ def _validate(self): f"'{cmd['name']}': {alias_reason}" ) + _collect_extension_command_names(ext["id"], commands) + # Rewrite any hook command references that pointed at a renamed command or # an alias-form ref (ext.cmd → speckit.ext.cmd). Always emit a warning when # the reference is changed so extension authors know to update the manifest. @@ -608,6 +724,15 @@ def _validate_provided_artifacts(entries: List[Any], section: str, singular: str ) seen_names.add(name) + # The name is an identifier component, so it may not contain the + # ':' delimiter. VALID_EXTENSION_ARTIFACT_NAME_PATTERN already + # excludes it; the explicit guard keeps the guarantee uniform with + # the hook fields. + try: + validate_component(name, f"{singular} name '{name}'") + except IdentifierComponentError as exc: + raise ValidationError(str(exc)) from exc + file_value = entry["file"] reason = relative_extension_path_violation(file_value) if reason: @@ -725,6 +850,91 @@ def hooks(self) -> Dict[str, Any]: """Get hook definitions.""" return self.data.get("hooks", {}) + def iter_contributions(self) -> List[Dict[str, Any]]: + """Return an enriched, ordered list of every contribution this manifest declares. + + Each dict is a shallow copy of the underlying manifest entry with four + derived keys added: ``layer`` (always ``"extension"``), ``sourceId`` + (this manifest's ``id``), ``kind`` (``"command"`` / ``"template"`` / + ``"script"`` / ``"hook"``), and ``id`` (the deterministic identifier). + Hook entries also carry a synthesized ``name`` field of the form + ``"{eventName}:{command}"`` alongside the original ``eventName`` / + ``command`` values, so consumers can locate a hook by its identifier's + name component without re-splitting the string. + + The underlying ``self.data`` mapping is never mutated — the enriched + dicts are constructed fresh on every call so callers can safely rely on + the identifiers reflecting the current in-memory manifest state. + """ + source_id = self.id + contributions: List[Dict[str, Any]] = [] + + for cmd in self.commands: + enriched = dict(cmd) + name = cmd.get("name", "") + enriched.update( + layer="extension", + sourceId=source_id, + kind="command", + id=derive_named_id("extension", source_id, "command", name), + ) + contributions.append(enriched) + + for tmpl in self.templates: + enriched = dict(tmpl) + name = tmpl.get("name", "") + enriched.update( + layer="extension", + sourceId=source_id, + kind="template", + id=derive_named_id("extension", source_id, "template", name), + ) + contributions.append(enriched) + + for scr in self.scripts: + enriched = dict(scr) + name = scr.get("name", "") + enriched.update( + layer="extension", + sourceId=source_id, + kind="script", + id=derive_named_id("extension", source_id, "script", name), + ) + contributions.append(enriched) + + hooks = self.hooks or {} + for event_name, hook_config in hooks.items(): + for entry in collapse_hook_event_entries(event_name, hook_config): + command_value = entry.get("command", "") + hook_id = derive_hook_id( + "extension", + source_id, + event_name, + command_value, + ) + enriched = dict(entry) + enriched.update( + layer="extension", + sourceId=source_id, + kind="hook", + name=f"{event_name}:{command_value}", + id=hook_id, + ) + contributions.append(enriched) + + return contributions + + def contribution_id(self, kind: str, name: str) -> Optional[str]: + """Return the computed identifier for a single contribution, if declared. + + ``name`` is the declared name for command/template/script kinds, or the + ``"{eventName}:{command}"`` compound for hook kinds. + """ + for entry in self.iter_contributions(): + if entry["kind"] == kind and entry.get("name") == name: + return entry["id"] + return None + def get_hash(self) -> str: """Calculate SHA256 hash of manifest file.""" h = hashlib.sha256() @@ -1092,67 +1302,7 @@ def _collect_manifest_command_names(manifest: ExtensionManifest) -> Dict[str, st Raises: ValidationError: If any declared name is invalid """ - if manifest.id in CORE_COMMAND_NAMES: - raise ValidationError( - f"Extension ID '{manifest.id}' conflicts with core command namespace '{manifest.id}'" - ) - - declared_names: Dict[str, str] = {} - - for cmd in manifest.commands: - primary_name = cmd["name"] - aliases = cmd.get("aliases", []) - - if aliases is None: - aliases = [] - if not isinstance(aliases, list): - raise ValidationError( - f"Aliases for command '{primary_name}' must be a list" - ) - - for kind, name in [("command", primary_name)] + [ - ("alias", alias) for alias in aliases - ]: - if not isinstance(name, str): - raise ValidationError( - f"{kind.capitalize()} for command '{primary_name}' must be a string" - ) - - path_reason = relative_extension_path_violation(name) - if path_reason: - raise ValidationError( - f"Invalid {kind} {name!r}: {path_reason}" - ) - - # Enforce canonical pattern only for primary command names; - # aliases are free-form to preserve community extension compat. - if kind == "command": - match = EXTENSION_COMMAND_NAME_PATTERN.match(name) - if match is None: - raise ValidationError( - f"Invalid {kind} '{name}': " - "must follow pattern 'speckit.{extension}.{command}'" - ) - - namespace = match.group(1) - if namespace != manifest.id: - raise ValidationError( - f"{kind.capitalize()} '{name}' must use extension namespace '{manifest.id}'" - ) - - if namespace in CORE_COMMAND_NAMES: - raise ValidationError( - f"{kind.capitalize()} '{name}' conflicts with core command namespace '{namespace}'" - ) - - if name in declared_names: - raise ValidationError( - f"Duplicate command or alias '{name}' in extension manifest" - ) - - declared_names[name] = kind - - return declared_names + return _collect_extension_command_names(manifest.id, manifest.commands) def _get_installed_command_name_map( self, @@ -5029,17 +5179,11 @@ def register_hooks(self, manifest: ExtensionManifest): config["hooks"][hook_name] = [] changed = True - # Key by command to dedup within the manifest. Deleting before - # re-insert moves a duplicate to the end so "last wins" also breaks ties. + # Key by command after canonical last-write-wins collapse so order + # exactly matches iter_contributions() for duplicate declarations. new_entries: Dict[str, Dict[str, Any]] = {} - for entry in coerce_hook_entries(hook_config): - if not isinstance(entry, dict): - continue - command = entry.get("command") - if not command: - continue - if command in new_entries: - del new_entries[command] + for entry in collapse_hook_event_entries(hook_name, hook_config): + command = entry["command"] new_entries[command] = { "extension": manifest.id, "command": command, diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index a5cea4f958..0aeab6e834 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -37,6 +37,12 @@ safe_extract_archive, ) from ..extensions import REINSTALL_COMMAND, ExtensionRegistry, normalize_priority +from .._identifier import ( + PROJECT_OVERRIDE_LAYER, + IdentifierComponentError, + derive_named_id, + validate_component, +) from .._init_options import ( MISSING_INIT_OPTIONS_FILE, is_ai_skills_enabled, @@ -499,6 +505,15 @@ def _validate(self): "must be lowercase alphanumeric with hyphens only" ) + # The name is an identifier component, so it may not contain the + # ':' delimiter. The patterns above already exclude it; the explicit + # guard keeps the guarantee uniform with the hook fields and holds + # if those patterns are ever relaxed. + try: + validate_component(tmpl["name"], f"template name '{tmpl['name']}'") + except IdentifierComponentError as exc: + raise PresetValidationError(str(exc)) from exc + @property def id(self) -> str: """Get preset ID.""" @@ -539,6 +554,38 @@ def tags(self) -> List[str]: """Get preset tags.""" return self.data.get("tags", []) + def iter_contributions(self) -> List[Dict[str, Any]]: + """Return an enriched, ordered list of every contribution this preset declares. + + Each dict is a shallow copy of the underlying ``provides.templates[]`` + entry with four derived keys added: ``layer`` (always ``"preset"``), + ``sourceId`` (this preset's ``id``), ``kind`` (mirrors the entry's + ``type`` — one of ``"command"`` / ``"template"`` / ``"script"``), and + ``id`` (the deterministic identifier). The underlying manifest data is + not mutated. + """ + source_id = self.id + contributions: List[Dict[str, Any]] = [] + for entry in self.templates: + kind = entry.get("type", "") + name = entry.get("name", "") + enriched = dict(entry) + enriched.update( + layer="preset", + sourceId=source_id, + kind=kind, + id=derive_named_id("preset", source_id, kind, name), + ) + contributions.append(enriched) + return contributions + + def contribution_id(self, kind: str, name: str) -> Optional[str]: + """Return the computed identifier for a single contribution, if declared.""" + for entry in self.iter_contributions(): + if entry["kind"] == kind and entry.get("name") == name: + return entry["id"] + return None + def get_hash(self) -> str: """Calculate SHA256 hash of manifest file.""" h = hashlib.sha256() @@ -5071,12 +5118,13 @@ def _manifest_declared_template( def _extension_manifest_declared_template( self, ext_dir: Path, template_name: str, template_type: str - ) -> tuple[dict | None, Path | None]: + ) -> tuple[dict | None, Path | None, str | None]: """Resolve an extension's manifest-declared command/template/script entry and usable file. - Mirrors ``_manifest_declared_template`` (for presets): returns ``(entry, candidate)`` - where ``entry`` is the matching ``provides.`` mapping, or ``None`` if the - extension has no (valid) manifest or doesn't declare this ``(name, type)``. + Mirrors ``_manifest_declared_template`` (for presets): returns + ``(entry, candidate, manifest_id)`` where ``entry`` is the matching + ``provides.`` mapping, or ``None`` if the extension has no (valid) + manifest or doesn't declare this ``(name, type)``. ``candidate`` is the declared ``file:`` resolved under ``ext_dir`` IFF it is a regular file that stays within ``ext_dir`` (guards against path traversal via a malformed manifest, mirroring ``resolve_extension_command_via_manifest``); @@ -5089,16 +5137,16 @@ def _extension_manifest_declared_template( diverge (the divergence flagged in review on #4012). """ if template_type not in ("command", "template", "script"): - return None, None + return None, None, None ext_manifest_path = ext_dir / "extension.yml" if not ext_manifest_path.exists(): - return None, None + return None, None, None from ..extensions import ExtensionManifest, ValidationError as ExtValidationError try: ext_manifest = ExtensionManifest(ext_manifest_path) except (ExtValidationError, yaml.YAMLError, OSError, TypeError, AttributeError): - return None, None + return None, None, None if template_type == "command": entries = ext_manifest.commands elif template_type == "template": @@ -5110,10 +5158,10 @@ def _extension_manifest_declared_template( continue file_rel = entry.get("file") if not file_rel: - return entry, None + return entry, None, ext_manifest.id rel_path = Path(file_rel) if rel_path.is_absolute(): - return entry, None + return entry, None, ext_manifest.id candidate = ext_dir / rel_path try: # Resolve only for the containment check, not for the @@ -5123,9 +5171,9 @@ def _extension_manifest_declared_template( # lookup returns for the same directory. candidate.resolve().relative_to(ext_dir.resolve()) # raises ValueError if outside except (OSError, ValueError): - return entry, None - return entry, (candidate if candidate.is_file() else None) - return None, None + return entry, None, ext_manifest.id + return entry, (candidate if candidate.is_file() else None), ext_manifest.id + return None, None, None def _get_all_extensions_by_priority(self) -> list[tuple[int, str, dict | None]]: """Build unified list of registered and unregistered extensions sorted by priority. @@ -5277,7 +5325,7 @@ def resolve( # The extension manifest is authoritative, same as preset manifests # above: check it before convention-based lookup so a declared entry # at a non-conventional path wins over a stale conventional file. - entry, manifest_candidate = self._extension_manifest_declared_template( + entry, manifest_candidate, _manifest_id = self._extension_manifest_declared_template( ext_dir, template_name, template_type ) if manifest_candidate is not None: @@ -5527,6 +5575,9 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: "path": override, "source": "project override", "strategy": "replace", + "lookupId": derive_named_id( + PROJECT_OVERRIDE_LAYER, "_", template_type, template_name + ), }) # Priority 2: Installed presets (sorted by priority — lower number = higher precedence) @@ -5583,6 +5634,9 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: "path": candidate, "source": f"{pack_id} v{version}", "strategy": strategy, + "lookupId": derive_named_id( + "preset", pack_id, template_type, template_name + ), }) # Priority 3: Extension-provided templates (always "replace") @@ -5594,7 +5648,7 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: # above: check it before convention-based lookup so a declared entry # at a non-conventional path wins over a stale conventional file, and # a declared-but-missing file isn't silently masked by convention. - entry, candidate = self._extension_manifest_declared_template( + entry, candidate, manifest_id = self._extension_manifest_declared_template( ext_dir, template_name, template_type ) if entry is None: @@ -5611,6 +5665,12 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: "strategy": "replace", "extension_id": ext_id, "extension_dir": ext_dir, + "lookupId": derive_named_id( + "extension", + manifest_id if entry is not None else ext_id, + template_type, + template_name, + ), }) # Priority 4: Core templates (always "replace") @@ -5639,6 +5699,9 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: "path": core, "source": "core", "strategy": "replace", + "lookupId": derive_named_id( + "core", "_", template_type, template_name + ), }) else: # Priority 5: Bundled core_pack (wheel install) or repo-root @@ -5649,6 +5712,9 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: "path": bundled, "source": "core (bundled)", "strategy": "replace", + "lookupId": derive_named_id( + "core", "_", template_type, template_name + ), }) return layers diff --git a/tests/test_contribution_ids.py b/tests/test_contribution_ids.py new file mode 100644 index 0000000000..52098d2980 --- /dev/null +++ b/tests/test_contribution_ids.py @@ -0,0 +1,437 @@ +"""Tests for the deterministic contribution-id and stack lookup-id feature. + +Every command / template / script / hook contribution surfaced by a preset or +extension manifest exposes a computed ``id`` derived from author-declared data +only, and every layer of a resolved artifact stack exposes a matching +``lookupId``. The scenarios below cover: the identifier grammar across every +``layer x kind`` combination, duplicate hook collapse behavior, component +validation, contribution lookup, resolver ``lookupId`` round-trips, and +non-persistence of computed identifiers. +""" + +from __future__ import annotations + +import copy +import json +from pathlib import Path + +import pytest +import yaml + +from specify_cli._identifier import ( + IdentifierComponentError, + PROJECT_OVERRIDE_LAYER, + derive_hook_id, + derive_named_id, + validate_component, +) +from specify_cli.extensions import ExtensionManifest, ValidationError +from specify_cli.presets import PresetManifest, PresetResolver, PresetValidationError + + +# --------------------------------------------------------------------------- +# Fixture builders (programmatic — no on-disk fixture tree) +# --------------------------------------------------------------------------- + + +def _preset_data(pack_id: str = "speckit-core") -> dict: + return { + "schema_version": "1.0", + "preset": { + "id": pack_id, + "name": pack_id, + "version": "1.0.0", + "description": "Fixture preset", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + {"type": "command", "name": "speckit.plan", "file": "commands/plan.md"}, + {"type": "template", "name": "spec-template", "file": "templates/spec.md"}, + {"type": "script", "name": "setup-plan", "file": "scripts/setup-plan.sh"}, + ] + }, + } + + +def _extension_data( + ext_id: str = "speckit-git", + hooks: dict | None = None, + with_commands: bool = True, + with_templates: bool = True, + with_scripts: bool = True, +) -> dict: + data = { + "schema_version": "1.0", + "extension": { + "id": ext_id, + "name": ext_id, + "version": "1.0.0", + "description": "Fixture extension", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": {}, + } + if with_commands: + data["provides"]["commands"] = [ + { + "name": f"speckit.{ext_id}.branch", + "file": "commands/branch.md", + "description": "Fixture command", + } + ] + if with_templates: + data["provides"]["templates"] = [ + {"name": "pr-body", "file": "templates/pr-body.md"} + ] + if with_scripts: + data["provides"]["scripts"] = [ + {"name": "post-commit", "file": "scripts/post-commit.sh"} + ] + if hooks is not None: + data["hooks"] = hooks + return data + + +def _write_manifest(tmp_path: Path, data: dict, filename: str) -> Path: + manifest_path = tmp_path / filename + with open(manifest_path, "w", encoding="utf-8") as fh: + yaml.safe_dump(data, fh, sort_keys=False) + return manifest_path + + +# --------------------------------------------------------------------------- +# Identifier grammar — layer x kind derivation matrix +# --------------------------------------------------------------------------- + + +class TestIdentifierDerivation: + """Every layer x kind combination produces the expected grammar.""" + + @pytest.mark.parametrize( + "layer, source_id, kind, name, expected", + [ + ("core", "_", "command", "speckit.constitution", "core:_:command:speckit.constitution"), + ("core", "_", "template", "spec-template", "core:_:template:spec-template"), + ("core", "_", "script", "setup-plan", "core:_:script:setup-plan"), + ("preset", "speckit-core", "command", "speckit.plan", "preset:speckit-core:command:speckit.plan"), + ("preset", "speckit-core", "template", "spec-template", "preset:speckit-core:template:spec-template"), + ("preset", "speckit-core", "script", "setup-plan", "preset:speckit-core:script:setup-plan"), + ("extension", "speckit-git", "command", "speckit.git.branch", "extension:speckit-git:command:speckit.git.branch"), + ("extension", "speckit-git", "template", "pr-body", "extension:speckit-git:template:pr-body"), + ("extension", "speckit-git", "script", "post-commit", "extension:speckit-git:script:post-commit"), + ], + ) + def test_named_id_grammar(self, layer, source_id, kind, name, expected): + assert derive_named_id(layer, source_id, kind, name) == expected + + @pytest.mark.parametrize( + "layer, source_id, event, command, expected", + [ + ("core", "_", "before_specify", "speckit.constitution", "core:_:hook:before_specify:speckit.constitution"), + ("preset", "speckit-core", "before_plan", "speckit.plan", "preset:speckit-core:hook:before_plan:speckit.plan"), + ("extension", "speckit-git", "before_specify", "speckit.git.branch", "extension:speckit-git:hook:before_specify:speckit.git.branch"), + ], + ) + def test_hook_id_grammar(self, layer, source_id, event, command, expected): + assert derive_hook_id(layer, source_id, event, command) == expected + + +# --------------------------------------------------------------------------- +# Duplicate hook behavior +# --------------------------------------------------------------------------- + + +class TestDuplicateHooks: + def test_last_duplicate_hook_is_the_only_contribution(self, tmp_path): + data = _extension_data( + hooks={ + "before_plan": [ + {"command": "speckit.speckit-git.branch", "priority": 10}, + {"command": "speckit.speckit-git.branch", "priority": 20}, + ] + } + ) + manifest = ExtensionManifest(_write_manifest(tmp_path, data, "extension.yml")) + hooks = [c for c in manifest.iter_contributions() if c["kind"] == "hook"] + assert len(hooks) == 1 + assert hooks[0]["priority"] == 20 + assert hooks[0]["id"] == "extension:speckit-git:hook:before_plan:speckit.speckit-git.branch" + assert ( + manifest.contribution_id( + "hook", "before_plan:speckit.speckit-git.branch" + ) + == hooks[0]["id"] + ) + + def test_duplicate_hook_moves_to_end_like_installer(self, tmp_path): + data = _extension_data( + hooks={ + "before_plan": [ + {"command": "speckit.dup", "priority": 1}, + {"command": "speckit.other", "priority": 2}, + {"command": "speckit.dup", "priority": 3}, + ] + }, + with_commands=False, + with_templates=False, + with_scripts=False, + ) + manifest = ExtensionManifest(_write_manifest(tmp_path, data, "extension.yml")) + hooks = [c for c in manifest.iter_contributions() if c["kind"] == "hook"] + assert [h["name"] for h in hooks] == [ + "before_plan:speckit.other", + "before_plan:speckit.dup", + ] + assert [h["priority"] for h in hooks] == [2, 3] + + +# --------------------------------------------------------------------------- +# Manifest component `:` guard +# --------------------------------------------------------------------------- + + +class TestComponentGuard: + def test_validate_component_rejects_colon(self): + with pytest.raises(IdentifierComponentError) as exc_info: + validate_component("has:colon", "test field") + assert "':' is reserved" in str(exc_info.value) + + def test_validate_component_rejects_empty(self): + with pytest.raises(IdentifierComponentError): + validate_component("", "test field") + + def test_validate_component_rejects_non_string(self): + with pytest.raises(IdentifierComponentError): + validate_component(42, "test field") + + def test_extension_hook_event_name_with_colon_rejected(self, tmp_path): + data = _extension_data( + hooks={"before:plan": {"command": "speckit.speckit-git.branch"}} + ) + with pytest.raises(ValidationError) as exc_info: + ExtensionManifest(_write_manifest(tmp_path, data, "extension.yml")) + assert "':' is reserved" in str(exc_info.value) + + def test_extension_hook_command_with_colon_rejected(self, tmp_path): + data = _extension_data( + hooks={"before_plan": {"command": "speckit:bad:command"}} + ) + with pytest.raises(ValidationError) as exc_info: + ExtensionManifest(_write_manifest(tmp_path, data, "extension.yml")) + assert "':' is reserved" in str(exc_info.value) + + @pytest.mark.parametrize("template_type", ["command", "template", "script"]) + def test_preset_template_name_with_colon_rejected(self, tmp_path, template_type): + data = _preset_data() + data["provides"]["templates"] = [ + {"type": template_type, "name": "bad:name", "file": "commands/x.md"} + ] + with pytest.raises(PresetValidationError): + PresetManifest(_write_manifest(tmp_path, data, "preset.yml")) + + @pytest.mark.parametrize("section", ["templates", "scripts"]) + def test_extension_artifact_name_with_colon_rejected(self, tmp_path, section): + data = _extension_data() + data["provides"][section] = [{"name": "bad:name", "file": "x/y.md"}] + with pytest.raises(ValidationError): + ExtensionManifest(_write_manifest(tmp_path, data, "extension.yml")) + + def test_extension_command_name_with_colon_rejected(self, tmp_path): + data = _extension_data() + data["provides"]["commands"] = [ + {"name": "speckit.git:branch", "file": "commands/branch.md"} + ] + with pytest.raises(ValidationError): + ExtensionManifest(_write_manifest(tmp_path, data, "extension.yml")) + + +# --------------------------------------------------------------------------- +# `iter_contributions` output surface +# --------------------------------------------------------------------------- + + +class TestContributionSurface: + def test_preset_iter_contributions_matrix(self, tmp_path): + manifest = PresetManifest(_write_manifest(tmp_path, _preset_data(), "preset.yml")) + entries = manifest.iter_contributions() + by_kind = {e["kind"]: e for e in entries} + assert by_kind["command"]["id"] == "preset:speckit-core:command:speckit.plan" + assert by_kind["template"]["id"] == "preset:speckit-core:template:spec-template" + assert by_kind["script"]["id"] == "preset:speckit-core:script:setup-plan" + for entry in entries: + assert entry["layer"] == "preset" + assert entry["sourceId"] == "speckit-core" + + def test_extension_iter_contributions_matrix(self, tmp_path): + data = _extension_data( + hooks={"before_specify": {"command": "speckit.speckit-git.branch"}} + ) + manifest = ExtensionManifest(_write_manifest(tmp_path, data, "extension.yml")) + entries = manifest.iter_contributions() + kinds = {e["kind"]: e for e in entries} + assert kinds["command"]["id"] == "extension:speckit-git:command:speckit.speckit-git.branch" + assert kinds["template"]["id"] == "extension:speckit-git:template:pr-body" + assert kinds["script"]["id"] == "extension:speckit-git:script:post-commit" + assert kinds["hook"]["id"] == "extension:speckit-git:hook:before_specify:speckit.speckit-git.branch" + assert kinds["hook"]["name"] == "before_specify:speckit.speckit-git.branch" + + def test_contribution_id_lookup(self, tmp_path): + manifest = PresetManifest(_write_manifest(tmp_path, _preset_data(), "preset.yml")) + assert ( + manifest.contribution_id("command", "speckit.plan") + == "preset:speckit-core:command:speckit.plan" + ) + assert manifest.contribution_id("command", "does-not-exist") is None + + def test_underlying_data_not_mutated(self, tmp_path): + original = _preset_data() + original_snapshot = copy.deepcopy(original) + manifest = PresetManifest(_write_manifest(tmp_path, original, "preset.yml")) + _ = manifest.iter_contributions() + assert manifest.data == original_snapshot + + +# --------------------------------------------------------------------------- +# `lookupId` round-trip through the resolver +# --------------------------------------------------------------------------- + + +def _make_project(root: Path) -> Path: + """Create a minimal project layout the resolver understands.""" + (root / ".specify" / "presets").mkdir(parents=True) + (root / ".specify" / "extensions").mkdir(parents=True) + (root / ".specify" / "memory").mkdir(parents=True) + (root / "templates" / "commands").mkdir(parents=True) + (root / "templates" / "scripts").mkdir(parents=True) + return root + + +class TestLookupIdRoundTrip: + def test_project_override_layer_carries_sentinel_lookup_id(self, tmp_path): + project = _make_project(tmp_path) + overrides_dir = project / ".specify" / "templates" / "overrides" + overrides_dir.mkdir(parents=True) + (overrides_dir / "spec-template.md").write_text("override", encoding="utf-8") + resolver = PresetResolver(project) + layers = resolver.collect_all_layers("spec-template", "template") + override_layer = next( + layer for layer in layers if layer["source"] == "project override" + ) + assert override_layer["lookupId"] == derive_named_id( + PROJECT_OVERRIDE_LAYER, "_", "template", "spec-template" + ) + + def test_core_layer_carries_core_lookup_id(self, tmp_path): + project = _make_project(tmp_path) + (project / "templates" / "spec-template.md").write_text("core", encoding="utf-8") + # PresetResolver reads templates from a bundled/repo path — point the + # resolver at the fixture project by monkey-patching the templates_dir. + resolver = PresetResolver(project) + resolver.templates_dir = project / "templates" + layers = resolver.collect_all_layers("spec-template", "template") + core_layer = next(layer for layer in layers if layer["source"] == "core") + assert core_layer["lookupId"] == "core:_:template:spec-template" + + def test_preset_layer_lookup_id_matches_manifest_contribution_id(self, tmp_path): + project = _make_project(tmp_path) + pack_id = "speckit-fixture" + pack_dir = project / ".specify" / "presets" / pack_id + (pack_dir / "templates").mkdir(parents=True) + (pack_dir / "templates" / "spec-template.md").write_text("preset", encoding="utf-8") + _write_manifest( + pack_dir, + { + "schema_version": "1.0", + "preset": { + "id": pack_id, + "name": pack_id, + "version": "1.0.0", + "description": "Fixture", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "template", + "name": "spec-template", + "file": "templates/spec-template.md", + } + ] + }, + }, + "preset.yml", + ) + registry = { + "schema_version": "1.0", + "presets": { + pack_id: {"version": "1.0.0", "priority": 10, "enabled": True} + }, + } + (project / ".specify" / "presets" / ".registry").write_text( + json.dumps(registry), encoding="utf-8" + ) + resolver = PresetResolver(project) + layers = resolver.collect_all_layers("spec-template", "template") + preset_layer = next( + layer for layer in layers if layer["source"].startswith(pack_id) + ) + manifest = PresetManifest(pack_dir / "preset.yml") + assert preset_layer["lookupId"] == manifest.contribution_id("template", "spec-template") + assert preset_layer["lookupId"] == f"preset:{pack_id}:template:spec-template" + + def test_extension_layer_uses_manifest_id_when_directory_name_differs(self, tmp_path): + project = _make_project(tmp_path) + ext_dir = project / ".specify" / "extensions" / "local-copy" + (ext_dir / "templates").mkdir(parents=True) + (ext_dir / "templates" / "pr-body.md").write_text("extension", encoding="utf-8") + manifest_path = _write_manifest( + ext_dir, + _extension_data( + ext_id="real-id", + with_commands=False, + with_scripts=False, + ), + "extension.yml", + ) + + resolver = PresetResolver(project) + layers = resolver.collect_all_layers("pr-body", "template") + extension_layer = next( + layer for layer in layers if layer["source"] == "extension:local-copy (unregistered)" + ) + manifest = ExtensionManifest(manifest_path) + + assert extension_layer["lookupId"] == manifest.contribution_id("template", "pr-body") + assert extension_layer["lookupId"] == "extension:real-id:template:pr-body" + + +# --------------------------------------------------------------------------- +# Identifiers never persisted +# --------------------------------------------------------------------------- + + +class TestNoPersistence: + def test_no_id_written_to_manifest_files(self, tmp_path): + data = _extension_data( + hooks={"before_specify": {"command": "speckit.speckit-git.branch"}} + ) + manifest_path = _write_manifest(tmp_path, data, "extension.yml") + # Read identifiers to force the derivation code path. + manifest = ExtensionManifest(manifest_path) + ids = [c["id"] for c in manifest.iter_contributions()] + assert ids # sanity check — feature actually ran + on_disk = manifest_path.read_text(encoding="utf-8") + assert ":command:" not in on_disk + assert ":template:" not in on_disk + assert ":script:" not in on_disk + assert ":hook:" not in on_disk + + def test_no_id_written_to_preset_manifest_files(self, tmp_path): + preset_path = _write_manifest(tmp_path, _preset_data(), "preset.yml") + manifest = PresetManifest(preset_path) + _ = [c["id"] for c in manifest.iter_contributions()] + on_disk = preset_path.read_text(encoding="utf-8") + assert ":command:" not in on_disk + assert ":template:" not in on_disk + assert ":script:" not in on_disk diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 6642da2b09..d1c2df8eb7 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -635,6 +635,51 @@ def test_alias_free_form_accepted(self, temp_dir, valid_manifest_data): assert manifest.commands[0]["aliases"] == ["speckit.hello"] assert manifest.warnings == [] + def test_duplicate_primary_command_rejected_at_manifest_load( + self, temp_dir, valid_manifest_data + ): + """Duplicate primary command names are rejected before install validation.""" + import yaml + + valid_manifest_data["provides"]["commands"].append( + { + "name": "speckit.test-ext.hello", + "file": "commands/hello-again.md", + } + ) + manifest_path = temp_dir / "extension.yml" + manifest_path.write_text(yaml.safe_dump(valid_manifest_data)) + + with pytest.raises( + ValidationError, + match="Duplicate command or alias 'speckit.test-ext.hello'", + ): + ExtensionManifest(manifest_path) + + def test_primary_command_alias_collision_rejected_at_manifest_load( + self, temp_dir, valid_manifest_data + ): + """A primary name cannot duplicate an alias from another command.""" + import yaml + + valid_manifest_data["provides"]["commands"][0]["aliases"] = [ + "speckit.test-ext.goodbye" + ] + valid_manifest_data["provides"]["commands"].append( + { + "name": "speckit.test-ext.goodbye", + "file": "commands/goodbye.md", + } + ) + manifest_path = temp_dir / "extension.yml" + manifest_path.write_text(yaml.safe_dump(valid_manifest_data)) + + with pytest.raises( + ValidationError, + match="Duplicate command or alias 'speckit.test-ext.goodbye'", + ): + ExtensionManifest(manifest_path) + def test_valid_command_name_has_no_warnings(self, temp_dir, valid_manifest_data): """Test that a correctly-named command produces no warnings.""" import yaml