From cf9c6b48794cad2d6c93be63c6e8e26f561e75f0 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 19:17:43 -0500 Subject: [PATCH 01/11] feat: expose resolver lookup IDs Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/artifacts.md | 2 +- src/specify_cli/_contribution_ids.py | 51 +++++++++++++++++++++++ src/specify_cli/artifacts/_identifiers.py | 51 ++++------------------- src/specify_cli/artifacts/resolution.py | 11 ++++- src/specify_cli/presets/__init__.py | 34 ++++++++++++++- tests/test_artifact_command.py | 14 ++++++- tests/test_artifact_command_parity.py | 2 +- 7 files changed, 115 insertions(+), 50 deletions(-) create mode 100644 src/specify_cli/_contribution_ids.py diff --git a/docs/reference/artifacts.md b/docs/reference/artifacts.md index 2832f24523..f6070f17d0 100644 --- a/docs/reference/artifacts.md +++ b/docs/reference/artifacts.md @@ -147,7 +147,7 @@ For command, template, and script artifacts, `stack` is ordered by resolution pr `active` and `hidden` are independent labels, not opposites. For command, template, and script artifacts, `active` identifies the highest-precedence layer selected by the existing Spec Kit layer-resolution order; it does not validate that the layer content can be read or composed. This preserves the diagnostic behavior of `specify preset resolve`, which reports the discovered layer chain even when content composition later produces a warning. Composing strategies (`wrap`, `prepend`, `append`) keep lower layers in the composed output, so an inactive layer is not necessarily hidden: only layers below the first `replace` layer are marked `hidden`. Built-in rows have no provenance: `layer`, `sourceId`, and `lookupId` are `null` — but `id` is always populated, even on built-in rows. `id` is the round-trip key: `specify artifact info` accepts it as input (for example, `specify artifact info command:speckit.specify --json`), and it resolves the same artifact whether the caller passes the bare name or the `id`. -Lookup IDs are derived by the artifact command from the resolved layer and its existing preset or extension manifest. Manifest-declared layers use the manifest's `id`; convention-only layers use the installed preset or extension directory id. Project-local overrides carry a synthetic `project:_:{kind}:{name}` ID, while built-in layers have no `lookupId`. These values are artifact-stack provenance, not the round-trip key — use `id` for that. `sourcePath` is populated only when the layer maps to a concrete installed preset/extension file or a tracked agent materialization; core, project-override, and other synthetic rows report `null`. +Lookup IDs are supplied by the resolver for contribution layers and reused by the artifact command. Manifest-declared layers use the manifest's `id`; convention-only layers use the installed preset or extension directory id. Project-local overrides carry a synthetic `project:_:{kind}:{name}` ID, while built-in layers have no `lookupId`. These values are artifact-stack provenance, not the round-trip key — use `id` for that. `sourcePath` is populated only when the layer maps to a concrete installed preset/extension file or a tracked agent materialization; core, project-override, and other synthetic rows report `null`. ### Hook artifacts diff --git a/src/specify_cli/_contribution_ids.py b/src/specify_cli/_contribution_ids.py new file mode 100644 index 0000000000..d30718c138 --- /dev/null +++ b/src/specify_cli/_contribution_ids.py @@ -0,0 +1,51 @@ +"""Shared computed identifiers for manifest-backed contributions.""" + +from __future__ import annotations + +from typing import Any + +PROJECT_OVERRIDE_LAYER = "project" +_ARTIFACT_KINDS = frozenset({"command", "template", "script"}) +_LAYER_KINDS = frozenset({PROJECT_OVERRIDE_LAYER, "preset", "extension"}) + + +class IdentifierComponentError(ValueError): + """Raised when a value cannot be represented in a contribution ID.""" + + +def validate_component(value: Any, field_label: str) -> str: + """Return a non-empty string without the ID delimiter.""" + 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_lookup_id(layer: str, source_id: str, kind: str, name: str) -> str: + """Build a deterministic manifest or stack contribution identifier.""" + validate_component(layer, "layer") + validate_component(source_id, "sourceId") + validate_component(kind, "kind") + validate_component(name, "name") + if layer not in _LAYER_KINDS: + raise IdentifierComponentError(f"Invalid layer '{layer}'") + if kind not in _ARTIFACT_KINDS: + raise IdentifierComponentError(f"Invalid artifact kind '{kind}'") + if layer == PROJECT_OVERRIDE_LAYER and source_id != "_": + raise IdentifierComponentError( + f"Invalid sourceId '{source_id}': project layer requires '_'" + ) + if layer != PROJECT_OVERRIDE_LAYER and source_id == "_": + raise IdentifierComponentError( + "Invalid sourceId '_': reserved for project layer" + ) + return f"{layer}:{source_id}:{kind}:{name}" diff --git a/src/specify_cli/artifacts/_identifiers.py b/src/specify_cli/artifacts/_identifiers.py index 7b5cfb644f..5e10c2f456 100644 --- a/src/specify_cli/artifacts/_identifiers.py +++ b/src/specify_cli/artifacts/_identifiers.py @@ -6,34 +6,18 @@ from typing import Any from urllib.parse import quote, unquote_to_bytes -PROJECT_OVERRIDE_LAYER = "project" +from .._contribution_ids import ( + PROJECT_OVERRIDE_LAYER, + IdentifierComponentError, + derive_lookup_id, + validate_component, +) + _ARTIFACT_KINDS = frozenset({"command", "template", "script"}) -_LAYER_KINDS = frozenset({PROJECT_OVERRIDE_LAYER, "preset", "extension"}) _HOOK_LAYERS = frozenset({"preset", "extension"}) _INVALID_PERCENT_ESCAPE = re.compile(r"%(?![0-9A-Fa-f]{2})") -class IdentifierComponentError(ValueError): - """Raised when a value cannot be represented in an artifact identifier.""" - - -def validate_component(value: Any, field_label: str) -> str: - """Return a non-empty string that does not contain the ID delimiter.""" - 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_public_id(kind: str, name: str) -> str: """Build the source-agnostic identifier exposed by ``specify artifact``.""" validate_component(kind, "kind") @@ -43,27 +27,6 @@ def derive_public_id(kind: str, name: str) -> str: return f"{kind}:{name}" -def derive_lookup_id(layer: str, source_id: str, kind: str, name: str) -> str: - """Build an artifact-stack lookup identifier.""" - validate_component(layer, "layer") - validate_component(source_id, "sourceId") - validate_component(kind, "kind") - validate_component(name, "name") - if layer not in _LAYER_KINDS: - raise IdentifierComponentError(f"Invalid layer '{layer}'") - if kind not in _ARTIFACT_KINDS: - raise IdentifierComponentError(f"Invalid artifact kind '{kind}'") - if layer == PROJECT_OVERRIDE_LAYER and source_id != "_": - raise IdentifierComponentError( - f"Invalid sourceId '{source_id}': project layer requires '_'" - ) - if layer != PROJECT_OVERRIDE_LAYER and source_id == "_": - raise IdentifierComponentError( - "Invalid sourceId '_': reserved for project layer" - ) - return f"{layer}:{source_id}:{kind}:{name}" - - def derive_hook_public_id(event_name: str, command: str) -> str: """Build the source-agnostic identifier for a hook artifact.""" encoded_event = _encode_hook_component(event_name, "eventName") diff --git a/src/specify_cli/artifacts/resolution.py b/src/specify_cli/artifacts/resolution.py index 73625a630f..c3742fa023 100644 --- a/src/specify_cli/artifacts/resolution.py +++ b/src/specify_cli/artifacts/resolution.py @@ -27,8 +27,11 @@ class _LayerProvenance: pack_dir: Path | None manifest: Any | None manifest_entry: dict[str, Any] | None + resolver_lookup_id: str | None = None def lookup_id(self, kind: ArtifactKind, name: str) -> str | None: + if self.resolver_lookup_id is not None: + return self.resolver_lookup_id if self.layer is None or self.source_id is None: return None try: @@ -88,9 +91,11 @@ def _layer_provenance( path = resolver_layer.get("path") if source == "project override": - return _LayerProvenance("project", "_", None, None, None, None) + return _LayerProvenance( + "project", "_", None, None, None, None, resolver_layer.get("lookupId") + ) if source in {"core", "core (bundled)"}: - return _LayerProvenance(None, None, None, None, None, None) + return _LayerProvenance(None, None, None, None, None, None, None) if not isinstance(path, Path) or not isinstance(source, str): raise ArtifactResolutionError() @@ -134,6 +139,7 @@ def _layer_provenance( extension_dir, manifest, declared, + resolver_layer.get("lookupId"), ) try: @@ -163,6 +169,7 @@ def _layer_provenance( pack_dir, manifest, declared, + resolver_layer.get("lookupId"), ) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index b22a440661..7e9ce1a6eb 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -36,6 +36,7 @@ read_response_limited, safe_extract_archive, ) +from .._contribution_ids import IdentifierComponentError, derive_lookup_id from ..extensions import REINSTALL_COMMAND, ExtensionRegistry, normalize_priority from .._init_options import ( MISSING_INIT_OPTIONS_FILE, @@ -5822,7 +5823,8 @@ def collect_all_layers( """Collect all layers in the priority stack for a template. Returns layers from highest priority (checked first) to lowest priority. - Each layer is a dict with 'path', 'source', and 'strategy' keys. + Each layer includes the established 'path', 'source', and 'strategy' + keys, plus a deterministic 'lookupId' for contribution layers. Args: template_name: Template name (e.g., "spec-template") @@ -5846,6 +5848,12 @@ def collect_all_layers( layers: List[Dict[str, Any]] = [] + def _lookup_id(layer: str, source_id: str) -> Optional[str]: + try: + return derive_lookup_id(layer, source_id, template_type, template_name) + except IdentifierComponentError: + return None + def _find_in_subdirs(base_dir: Path) -> Optional[Path]: for subdir in subdirs: if subdir: @@ -5866,6 +5874,7 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: "path": override, "source": "project override", "strategy": "replace", + "lookupId": _lookup_id("project", "_"), }) # Priority 2: Installed presets (sorted by priority — lower number = higher precedence) @@ -5918,10 +5927,15 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: # strategy ("replace") when content is unreadable/invalid. pass version = metadata.get("version", "?") if metadata else "?" + manifest = self._get_manifest(pack_dir) layers.append({ "path": candidate, "source": f"{pack_id} v{version}", "strategy": strategy, + "lookupId": _lookup_id( + "preset", + manifest.id if manifest is not None and entry is not None else pack_id, + ), }) # Priority 3: Extension-provided templates (always "replace") @@ -5944,12 +5958,30 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: source = f"extension:{ext_id} v{version}" else: source = f"extension:{ext_id} (unregistered)" + from ..extensions import ( + ExtensionManifest, + ValidationError as ExtValidationError, + ) + + source_id = ext_id + try: + if entry is not None: + source_id = ExtensionManifest(ext_dir / "extension.yml").id + except ( + ExtValidationError, + yaml.YAMLError, + OSError, + TypeError, + AttributeError, + ): + pass layers.append({ "path": candidate, "source": source, "strategy": "replace", "extension_id": ext_id, "extension_dir": ext_dir, + "lookupId": _lookup_id("extension", source_id), }) # Priority 4: Core templates (always "replace") diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index 751983f6a9..45c6252b4e 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -372,7 +372,7 @@ def test_unregistered_extension_manifest_id_wins_for_lookup(self, spec_kit_proje resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( "speckit.original.hello", "command" )[0] - assert "lookupId" not in resolver_layer + assert resolver_layer["lookupId"] == info["stack"][0]["lookupId"] # The stack row's manifestPath must still reflect the actual on-disk # extension directory (``renamed``), not the manifest id embedded in # ``lookupId``. @@ -383,10 +383,14 @@ def test_unregistered_extension_manifest_id_wins_for_lookup(self, spec_kit_proje convention = catalog.get_artifact_info("speckit.renamed.convention")[ "stack" ][0] + convention_layer = PresetResolver(spec_kit_project).collect_all_layers( + "speckit.renamed.convention", "command" + )[0] assert convention["sourceId"] == "renamed" assert convention["lookupId"] == ( "extension:renamed:command:speckit.renamed.convention" ) + assert convention_layer["lookupId"] == convention["lookupId"] assert convention["manifestPath"] is None def test_includes_project_local_core_assets(self, spec_kit_project: Path): @@ -659,6 +663,10 @@ def test_project_override_row_shape(self, spec_kit_project: Path): assert project["strategy"] == "replace" assert project["sourceId"] == "_" assert re.match(r"^project:_:(command|template|script):[^:]+$", project["lookupId"]) + resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( + "speckit.constitution", "command" + )[0] + assert resolver_layer["lookupId"] == project["lookupId"] def test_lookup_id_grammar(self, spec_kit_project: Path): info = ArtifactCatalog(spec_kit_project).get_artifact_info("speckit.constitution") @@ -1555,9 +1563,13 @@ def test_unregistered_preset_template_without_manifest(self, spec_kit_project: P row.id == "template:legacy-preset-template" for row in catalog.list_artifacts() ) info = catalog.get_artifact_info("legacy-preset-template") + resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( + "legacy-preset-template", "template" + )[0] assert info["stack"][0]["lookupId"] == ( "preset:legacy-preset:template:legacy-preset-template" ) + assert resolver_layer["lookupId"] == info["stack"][0]["lookupId"] def test_stale_registry_entry_with_missing_pack_dir_is_skipped( self, spec_kit_project: Path diff --git a/tests/test_artifact_command_parity.py b/tests/test_artifact_command_parity.py index 00eeb85619..636c301a4f 100644 --- a/tests/test_artifact_command_parity.py +++ b/tests/test_artifact_command_parity.py @@ -104,7 +104,7 @@ def test_preset_manifest_id_mismatch_uses_manifest_id(self, spec_kit_project: Pa resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( "speckit.preset-renamed.hello", "command" )[0] - assert "lookupId" not in resolver_layer + assert resolver_layer["lookupId"] == active["lookupId"] # The stack row's presetId / manifestPath must still reflect the # actual on-disk directory (``renamed-preset``), not the manifest id # embedded in ``lookupId`` — otherwise the display and manifest path From 1f54e3f29d59850f761377040882b7d5307cc367 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 19:43:22 -0500 Subject: [PATCH 02/11] refactor: keep contribution lookup artifact-owned Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/artifacts.md | 21 ++- src/specify_cli/_contribution_ids.py | 51 -------- src/specify_cli/artifacts/__init__.py | 2 + src/specify_cli/artifacts/_commands.py | 25 ++++ src/specify_cli/artifacts/_identifiers.py | 76 ++++++++++- src/specify_cli/artifacts/catalog.py | 149 ++++++++++++++++++++++ src/specify_cli/artifacts/models.py | 7 + src/specify_cli/artifacts/resolution.py | 11 +- src/specify_cli/presets/__init__.py | 34 +---- tests/test_artifact_command.py | 122 ++++++++++++++++-- tests/test_artifact_command_parity.py | 7 +- 11 files changed, 388 insertions(+), 117 deletions(-) delete mode 100644 src/specify_cli/_contribution_ids.py diff --git a/docs/reference/artifacts.md b/docs/reference/artifacts.md index f6070f17d0..80604e368b 100644 --- a/docs/reference/artifacts.md +++ b/docs/reference/artifacts.md @@ -4,7 +4,7 @@ An **artifact** is any command, template, script, or hook Spec Kit exposes in a The `specify artifact` command group is the read-only introspection surface for that inventory. `specify preset resolve ` answers "which file wins for this preset-managed name?"; `specify artifact` answers "what exists at all, and what is the full composition stack behind it?" — including built-in artifacts that no preset touches. -Both subcommands currently require `--json`. Omitting it exits with code `2` and prints a usage message on stderr; no stdout is produced. Text rendering is deliberately deferred so the JSON shapes below are the only contract, and adding a default text renderer later stays a non-breaking, additive change. +All subcommands currently require `--json`. Omitting it exits with code `2` and prints a usage message on stderr; no stdout is produced. Text rendering is deliberately deferred so the JSON shapes below are the only contract, and adding a default text renderer later stays a non-breaking, additive change. ## List Artifacts @@ -147,7 +147,24 @@ For command, template, and script artifacts, `stack` is ordered by resolution pr `active` and `hidden` are independent labels, not opposites. For command, template, and script artifacts, `active` identifies the highest-precedence layer selected by the existing Spec Kit layer-resolution order; it does not validate that the layer content can be read or composed. This preserves the diagnostic behavior of `specify preset resolve`, which reports the discovered layer chain even when content composition later produces a warning. Composing strategies (`wrap`, `prepend`, `append`) keep lower layers in the composed output, so an inactive layer is not necessarily hidden: only layers below the first `replace` layer are marked `hidden`. Built-in rows have no provenance: `layer`, `sourceId`, and `lookupId` are `null` — but `id` is always populated, even on built-in rows. `id` is the round-trip key: `specify artifact info` accepts it as input (for example, `specify artifact info command:speckit.specify --json`), and it resolves the same artifact whether the caller passes the bare name or the `id`. -Lookup IDs are supplied by the resolver for contribution layers and reused by the artifact command. Manifest-declared layers use the manifest's `id`; convention-only layers use the installed preset or extension directory id. Project-local overrides carry a synthetic `project:_:{kind}:{name}` ID, while built-in layers have no `lookupId`. These values are artifact-stack provenance, not the round-trip key — use `id` for that. `sourcePath` is populated only when the layer maps to a concrete installed preset/extension file or a tracked agent materialization; core, project-override, and other synthetic rows report `null`. +Lookup IDs are derived by the artifact command from the resolved layer and its existing preset or extension manifest. Manifest-declared layers use the manifest's `id`; convention-only layers use the installed preset or extension directory id. Project-local overrides carry a synthetic `project:_:{kind}:{name}` ID, while built-in layers have no `lookupId`. These values are artifact-stack provenance, not the round-trip key — use `id` for that. `sourcePath` is populated only when the layer maps to a concrete installed preset/extension file or a tracked agent materialization; core, project-override, and other synthetic rows report `null`. + +## Contribution Lookup + +```bash +specify artifact lookup --json +``` + +Resolves a manifest-backed stack `lookupId` to the exact preset or extension +declaration that produced it. This keeps cross-reference behavior inside the +artifact command: preset and extension commands, manifests, and resolver return +shapes are unchanged. + +The response includes the stable lookup ID, provider coordinates, manifest and +source paths, and the original declaration under `contribution`. Convention-only +contributions and project overrides have no originating manifest declaration, +so lookup returns `{"error": "unknown contribution "}` with exit code +`1`. Built-in rows never have a `lookupId`. ### Hook artifacts diff --git a/src/specify_cli/_contribution_ids.py b/src/specify_cli/_contribution_ids.py deleted file mode 100644 index d30718c138..0000000000 --- a/src/specify_cli/_contribution_ids.py +++ /dev/null @@ -1,51 +0,0 @@ -"""Shared computed identifiers for manifest-backed contributions.""" - -from __future__ import annotations - -from typing import Any - -PROJECT_OVERRIDE_LAYER = "project" -_ARTIFACT_KINDS = frozenset({"command", "template", "script"}) -_LAYER_KINDS = frozenset({PROJECT_OVERRIDE_LAYER, "preset", "extension"}) - - -class IdentifierComponentError(ValueError): - """Raised when a value cannot be represented in a contribution ID.""" - - -def validate_component(value: Any, field_label: str) -> str: - """Return a non-empty string without the ID delimiter.""" - 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_lookup_id(layer: str, source_id: str, kind: str, name: str) -> str: - """Build a deterministic manifest or stack contribution identifier.""" - validate_component(layer, "layer") - validate_component(source_id, "sourceId") - validate_component(kind, "kind") - validate_component(name, "name") - if layer not in _LAYER_KINDS: - raise IdentifierComponentError(f"Invalid layer '{layer}'") - if kind not in _ARTIFACT_KINDS: - raise IdentifierComponentError(f"Invalid artifact kind '{kind}'") - if layer == PROJECT_OVERRIDE_LAYER and source_id != "_": - raise IdentifierComponentError( - f"Invalid sourceId '{source_id}': project layer requires '_'" - ) - if layer != PROJECT_OVERRIDE_LAYER and source_id == "_": - raise IdentifierComponentError( - "Invalid sourceId '_': reserved for project layer" - ) - return f"{layer}:{source_id}:{kind}:{name}" diff --git a/src/specify_cli/artifacts/__init__.py b/src/specify_cli/artifacts/__init__.py index d704c3a023..8b1790b76c 100644 --- a/src/specify_cli/artifacts/__init__.py +++ b/src/specify_cli/artifacts/__init__.py @@ -8,6 +8,7 @@ ArtifactKind, ArtifactNotFoundError, ArtifactResolutionError, + ContributionNotFoundError, HookArtifact, HookLayerName, HookStackEntry, @@ -25,6 +26,7 @@ "ArtifactKind", "ArtifactNotFoundError", "ArtifactResolutionError", + "ContributionNotFoundError", "HookArtifact", "HookLayerName", "HookStackEntry", diff --git a/src/specify_cli/artifacts/_commands.py b/src/specify_cli/artifacts/_commands.py index 986ca2edf0..66bea8e18a 100644 --- a/src/specify_cli/artifacts/_commands.py +++ b/src/specify_cli/artifacts/_commands.py @@ -158,6 +158,31 @@ def artifact_info( sys.stdout.write("\n") +@artifact_app.command("lookup") +def artifact_lookup( + lookup_id: str = typer.Argument(..., help="Contribution lookupId from an artifact stack."), + json_flag: bool = typer.Option( + False, + "--json", + help="Emit the originating manifest contribution as JSON.", + ), +) -> None: + """Resolve a stack lookupId to its preset or extension contribution.""" + _require_json_flag(json_flag) + try: + root = _resolve_project_root() + payload = ArtifactCatalog(root).get_contribution_info(lookup_id) + except ArtifactError as exc: + _emit_error_and_exit(exc) + return # pragma: no cover + except (OSError, PresetError): + _emit_error_and_exit(ArtifactResolutionError()) + return # pragma: no cover + + sys.stdout.write(json.dumps(payload, indent=2, sort_keys=True, ensure_ascii=False)) + sys.stdout.write("\n") + + def register(app: typer.Typer) -> None: """Attach the artifact command group to the root Typer app.""" app.add_typer(artifact_app, name="artifact") diff --git a/src/specify_cli/artifacts/_identifiers.py b/src/specify_cli/artifacts/_identifiers.py index 5e10c2f456..0da6499ac8 100644 --- a/src/specify_cli/artifacts/_identifiers.py +++ b/src/specify_cli/artifacts/_identifiers.py @@ -6,18 +6,34 @@ from typing import Any from urllib.parse import quote, unquote_to_bytes -from .._contribution_ids import ( - PROJECT_OVERRIDE_LAYER, - IdentifierComponentError, - derive_lookup_id, - validate_component, -) - +PROJECT_OVERRIDE_LAYER = "project" _ARTIFACT_KINDS = frozenset({"command", "template", "script"}) +_LAYER_KINDS = frozenset({PROJECT_OVERRIDE_LAYER, "preset", "extension"}) _HOOK_LAYERS = frozenset({"preset", "extension"}) _INVALID_PERCENT_ESCAPE = re.compile(r"%(?![0-9A-Fa-f]{2})") +class IdentifierComponentError(ValueError): + """Raised when a value cannot be represented in an artifact identifier.""" + + +def validate_component(value: Any, field_label: str) -> str: + """Return a non-empty string that does not contain the ID delimiter.""" + 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_public_id(kind: str, name: str) -> str: """Build the source-agnostic identifier exposed by ``specify artifact``.""" validate_component(kind, "kind") @@ -27,6 +43,27 @@ def derive_public_id(kind: str, name: str) -> str: return f"{kind}:{name}" +def derive_lookup_id(layer: str, source_id: str, kind: str, name: str) -> str: + """Build an artifact-stack lookup identifier.""" + validate_component(layer, "layer") + validate_component(source_id, "sourceId") + validate_component(kind, "kind") + validate_component(name, "name") + if layer not in _LAYER_KINDS: + raise IdentifierComponentError(f"Invalid layer '{layer}'") + if kind not in _ARTIFACT_KINDS: + raise IdentifierComponentError(f"Invalid artifact kind '{kind}'") + if layer == PROJECT_OVERRIDE_LAYER and source_id != "_": + raise IdentifierComponentError( + f"Invalid sourceId '{source_id}': project layer requires '_'" + ) + if layer != PROJECT_OVERRIDE_LAYER and source_id == "_": + raise IdentifierComponentError( + "Invalid sourceId '_': reserved for project layer" + ) + return f"{layer}:{source_id}:{kind}:{name}" + + def derive_hook_public_id(event_name: str, command: str) -> str: """Build the source-agnostic identifier for a hook artifact.""" encoded_event = _encode_hook_component(event_name, "eventName") @@ -62,6 +99,31 @@ def parse_hook_artifact_name(name: str) -> tuple[str, str]: ) +def parse_lookup_id(value: str) -> tuple[str, str, str, str]: + """Parse a contribution lookup ID into layer, source, kind, and name.""" + if not isinstance(value, str): + raise IdentifierComponentError("Invalid lookupId") + parts = value.split(":") + if len(parts) == 4: + layer, source_id, kind, name = parts + derive_lookup_id(layer, source_id, kind, name) + return layer, source_id, kind, name + if len(parts) == 5 and parts[2] == "hook": + layer, source_id, kind, encoded_event, encoded_command = parts + if layer not in _HOOK_LAYERS or source_id == "_": + raise IdentifierComponentError("Invalid hook lookupId") + event_name, command = parse_hook_artifact_name( + f"{encoded_event}:{encoded_command}" + ) + if ( + derive_hook_lookup_id(layer, source_id, event_name, command) + != value + ): + raise IdentifierComponentError("Invalid hook lookupId") + return layer, source_id, kind, f"{encoded_event}:{encoded_command}" + raise IdentifierComponentError("Invalid lookupId") + + def _encode_hook_component(value: Any, field_label: str) -> str: """Encode one hook ID component without narrowing manifest syntax.""" if not isinstance(value, str): diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index 8180a32e27..ecb6f9424f 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -25,6 +25,7 @@ derive_hook_public_id, derive_public_id, parse_hook_artifact_name, + parse_lookup_id, validate_component, ) from .models import ( @@ -33,6 +34,7 @@ ArtifactKind, ArtifactNotFoundError, ArtifactResolutionError, + ContributionNotFoundError, HookArtifact, HookStackEntry, NotASpecKitProjectError, @@ -405,6 +407,153 @@ def get_artifact_info( "stack": [layer.to_json_dict() for layer in stack], } + def get_contribution_info(self, lookup_id: str) -> dict[str, Any]: + """Resolve a stack ``lookupId`` to its originating manifest entry.""" + _validate_project(self.project_root) + _validate_extension_registry(self.project_root) + try: + layer, source_id, kind, name = parse_lookup_id(lookup_id) + except IdentifierComponentError as exc: + raise ContributionNotFoundError(lookup_id) from exc + if layer == "project": + raise ContributionNotFoundError(lookup_id) + + from ..presets import PresetError, PresetResolver + + resolver = PresetResolver(self.project_root) + try: + if layer == "preset": + resolved = self._find_preset_contribution( + resolver, source_id, kind, name + ) + else: + resolved = self._find_extension_contribution( + resolver, source_id, kind, name + ) + except (OSError, PresetError) as exc: + raise ArtifactResolutionError() from exc + if resolved is None: + raise ContributionNotFoundError(lookup_id) + + contribution, manifest_path, source_path = resolved + return { + "id": lookup_id, + "layer": layer, + "sourceId": source_id, + "kind": kind, + "name": name, + "manifestPath": manifest_path, + "sourcePath": source_path, + "contribution": contribution, + } + + def _find_preset_contribution( + self, + resolver: Any, + source_id: str, + kind: str, + name: str, + ) -> tuple[dict[str, Any], str, str | None] | None: + for pack_id, _metadata in resolver._get_all_presets_by_priority(): + pack_dir = resolver.presets_dir / pack_id + manifest = resolver._get_manifest(pack_dir) + if manifest is None or manifest.id != source_id: + continue + for entry in manifest.templates: + if ( + isinstance(entry, dict) + and entry.get("type") == kind + and entry.get("name") == name + ): + return self._contribution_result( + entry, pack_dir, manifest.path + ) + return None + + def _find_extension_contribution( + self, + resolver: Any, + source_id: str, + kind: str, + name: str, + ) -> tuple[dict[str, Any], str, str | None] | None: + from ..extensions import ( + ExtensionManifest, + ValidationError, + coerce_hook_entries, + ) + + for _priority, extension_id, _metadata in resolver._get_all_extensions_by_priority(): + extension_dir = resolver.extensions_dir / extension_id + manifest_path = extension_dir / "extension.yml" + try: + manifest = ExtensionManifest(manifest_path) + except (ValidationError, OSError, TypeError, AttributeError): + continue + if manifest.id != source_id: + continue + if kind == "hook": + event_name, command = parse_hook_artifact_name(name) + matching: dict[str, Any] | None = None + hook_config = (manifest.hooks or {}).get(event_name) + for entry in coerce_hook_entries(hook_config): + if isinstance(entry, dict) and entry.get("command") == command: + matching = entry + if matching is None: + return None + relative_manifest = _repo_relative_existing_file( + self.project_root, manifest.path + ) + if relative_manifest is None: + raise ArtifactResolutionError() + return ( + {"eventName": event_name, **matching}, + relative_manifest, + None, + ) + + entries = { + "command": manifest.commands, + "template": manifest.templates, + "script": manifest.scripts, + }[kind] + for entry in entries: + if isinstance(entry, dict) and entry.get("name") == name: + return self._contribution_result( + entry, extension_dir, manifest.path + ) + return None + + def _contribution_result( + self, + entry: dict[str, Any], + pack_dir: Path, + manifest_path: Path, + ) -> tuple[dict[str, Any], str, str | None]: + relative_manifest = _repo_relative_existing_file( + self.project_root, manifest_path + ) + if relative_manifest is None: + raise ArtifactResolutionError() + relative_file = entry.get("file") + source_path = None + if isinstance(relative_file, str): + try: + resolved_pack = pack_dir.resolve() + candidate = (pack_dir / relative_file).resolve() + candidate.relative_to(resolved_pack) + except (OSError, ValueError): + pass + else: + source_path = _repo_relative_existing_file( + self.project_root, candidate + ) + return ( + dict(entry), + relative_manifest, + source_path, + ) + def _get_hook_info( self, bare_name: str, diff --git a/src/specify_cli/artifacts/models.py b/src/specify_cli/artifacts/models.py index b8125017de..9d012830da 100644 --- a/src/specify_cli/artifacts/models.py +++ b/src/specify_cli/artifacts/models.py @@ -139,6 +139,12 @@ def __init__(self, name: str) -> None: super().__init__(self.message) +class ContributionNotFoundError(ArtifactError): + def __init__(self, lookup_id: str) -> None: + self.message = f"unknown contribution {lookup_id}" + super().__init__(self.message) + + class AmbiguousArtifactError(ArtifactError): def __init__(self, name: str, kinds: Iterable[str]) -> None: kinds_list = sorted(kinds) @@ -165,6 +171,7 @@ def __init__(self) -> None: "ArtifactKind", "ArtifactNotFoundError", "ArtifactResolutionError", + "ContributionNotFoundError", "HookArtifact", "HookLayerName", "HookStackEntry", diff --git a/src/specify_cli/artifacts/resolution.py b/src/specify_cli/artifacts/resolution.py index c3742fa023..73625a630f 100644 --- a/src/specify_cli/artifacts/resolution.py +++ b/src/specify_cli/artifacts/resolution.py @@ -27,11 +27,8 @@ class _LayerProvenance: pack_dir: Path | None manifest: Any | None manifest_entry: dict[str, Any] | None - resolver_lookup_id: str | None = None def lookup_id(self, kind: ArtifactKind, name: str) -> str | None: - if self.resolver_lookup_id is not None: - return self.resolver_lookup_id if self.layer is None or self.source_id is None: return None try: @@ -91,11 +88,9 @@ def _layer_provenance( path = resolver_layer.get("path") if source == "project override": - return _LayerProvenance( - "project", "_", None, None, None, None, resolver_layer.get("lookupId") - ) + return _LayerProvenance("project", "_", None, None, None, None) if source in {"core", "core (bundled)"}: - return _LayerProvenance(None, None, None, None, None, None, None) + return _LayerProvenance(None, None, None, None, None, None) if not isinstance(path, Path) or not isinstance(source, str): raise ArtifactResolutionError() @@ -139,7 +134,6 @@ def _layer_provenance( extension_dir, manifest, declared, - resolver_layer.get("lookupId"), ) try: @@ -169,7 +163,6 @@ def _layer_provenance( pack_dir, manifest, declared, - resolver_layer.get("lookupId"), ) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 7e9ce1a6eb..b22a440661 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -36,7 +36,6 @@ read_response_limited, safe_extract_archive, ) -from .._contribution_ids import IdentifierComponentError, derive_lookup_id from ..extensions import REINSTALL_COMMAND, ExtensionRegistry, normalize_priority from .._init_options import ( MISSING_INIT_OPTIONS_FILE, @@ -5823,8 +5822,7 @@ def collect_all_layers( """Collect all layers in the priority stack for a template. Returns layers from highest priority (checked first) to lowest priority. - Each layer includes the established 'path', 'source', and 'strategy' - keys, plus a deterministic 'lookupId' for contribution layers. + Each layer is a dict with 'path', 'source', and 'strategy' keys. Args: template_name: Template name (e.g., "spec-template") @@ -5848,12 +5846,6 @@ def collect_all_layers( layers: List[Dict[str, Any]] = [] - def _lookup_id(layer: str, source_id: str) -> Optional[str]: - try: - return derive_lookup_id(layer, source_id, template_type, template_name) - except IdentifierComponentError: - return None - def _find_in_subdirs(base_dir: Path) -> Optional[Path]: for subdir in subdirs: if subdir: @@ -5874,7 +5866,6 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: "path": override, "source": "project override", "strategy": "replace", - "lookupId": _lookup_id("project", "_"), }) # Priority 2: Installed presets (sorted by priority — lower number = higher precedence) @@ -5927,15 +5918,10 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: # strategy ("replace") when content is unreadable/invalid. pass version = metadata.get("version", "?") if metadata else "?" - manifest = self._get_manifest(pack_dir) layers.append({ "path": candidate, "source": f"{pack_id} v{version}", "strategy": strategy, - "lookupId": _lookup_id( - "preset", - manifest.id if manifest is not None and entry is not None else pack_id, - ), }) # Priority 3: Extension-provided templates (always "replace") @@ -5958,30 +5944,12 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]: source = f"extension:{ext_id} v{version}" else: source = f"extension:{ext_id} (unregistered)" - from ..extensions import ( - ExtensionManifest, - ValidationError as ExtValidationError, - ) - - source_id = ext_id - try: - if entry is not None: - source_id = ExtensionManifest(ext_dir / "extension.yml").id - except ( - ExtValidationError, - yaml.YAMLError, - OSError, - TypeError, - AttributeError, - ): - pass layers.append({ "path": candidate, "source": source, "strategy": "replace", "extension_id": ext_id, "extension_dir": ext_dir, - "lookupId": _lookup_id("extension", source_id), }) # Priority 4: Core templates (always "replace") diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index 45c6252b4e..2dee40e521 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -25,6 +25,7 @@ ArtifactKind, ArtifactNotFoundError, ArtifactResolutionError, + ContributionNotFoundError, HookArtifact, NotASpecKitProjectError, ) @@ -34,7 +35,8 @@ from tests.conftest import install_preset ERROR_REGEX = re.compile( - r"^(unknown artifact |ambiguous artifact |artifact resolution failed|not a Spec Kit project)" + r"^(unknown artifact |unknown contribution |ambiguous artifact |" + r"artifact resolution failed|not a Spec Kit project)" ) @@ -372,7 +374,7 @@ def test_unregistered_extension_manifest_id_wins_for_lookup(self, spec_kit_proje resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( "speckit.original.hello", "command" )[0] - assert resolver_layer["lookupId"] == info["stack"][0]["lookupId"] + assert "lookupId" not in resolver_layer # The stack row's manifestPath must still reflect the actual on-disk # extension directory (``renamed``), not the manifest id embedded in # ``lookupId``. @@ -380,18 +382,26 @@ def test_unregistered_extension_manifest_id_wins_for_lookup(self, spec_kit_proje info["stack"][0]["manifestPath"] == ".specify/extensions/renamed/extension.yml" ) + contribution = catalog.get_contribution_info( + info["stack"][0]["lookupId"] + ) + assert contribution["id"] == info["stack"][0]["lookupId"] + assert contribution["layer"] == "extension" + assert contribution["sourceId"] == "original" + assert contribution["kind"] == "command" + assert contribution["name"] == "speckit.original.hello" + assert contribution["contribution"]["file"] == "commands/actual.md" + convention = catalog.get_artifact_info("speckit.renamed.convention")[ "stack" ][0] - convention_layer = PresetResolver(spec_kit_project).collect_all_layers( - "speckit.renamed.convention", "command" - )[0] assert convention["sourceId"] == "renamed" assert convention["lookupId"] == ( "extension:renamed:command:speckit.renamed.convention" ) - assert convention_layer["lookupId"] == convention["lookupId"] assert convention["manifestPath"] is None + with pytest.raises(ContributionNotFoundError): + catalog.get_contribution_info(convention["lookupId"]) def test_includes_project_local_core_assets(self, spec_kit_project: Path): templates_dir = spec_kit_project / ".specify" / "templates" @@ -663,10 +673,6 @@ def test_project_override_row_shape(self, spec_kit_project: Path): assert project["strategy"] == "replace" assert project["sourceId"] == "_" assert re.match(r"^project:_:(command|template|script):[^:]+$", project["lookupId"]) - resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( - "speckit.constitution", "command" - )[0] - assert resolver_layer["lookupId"] == project["lookupId"] def test_lookup_id_grammar(self, spec_kit_project: Path): info = ArtifactCatalog(spec_kit_project).get_artifact_info("speckit.constitution") @@ -884,6 +890,86 @@ def test_list_json_rows_include_stack(self, spec_kit_project: Path, monkeypatch: info = json.loads(info_result.stdout) assert row["stack"] == info["stack"] + def test_lookup_json_cross_references_manifest_contribution( + self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch + ): + monkeypatch.chdir(spec_kit_project) + pack = install_preset( + spec_kit_project, + "lookup-pack", + { + "templates": [ + { + "type": "template", + "name": "lookup-template", + "file": "templates/lookup.md", + "description": "Lookup target", + } + ] + }, + ) + (pack / "templates").mkdir() + (pack / "templates" / "lookup.md").write_text("body", encoding="utf-8") + + lookup_id = ArtifactCatalog(spec_kit_project).get_artifact_info( + "template:lookup-template" + )["stack"][0]["lookupId"] + result = CliRunner().invoke( + app, ["artifact", "lookup", lookup_id, "--json"] + ) + + assert result.exit_code == 0, result.stderr + payload = json.loads(result.stdout) + assert payload["id"] == lookup_id + assert payload["contribution"]["description"] == "Lookup target" + + def test_lookup_json_rejects_unknown_contribution( + self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch + ): + monkeypatch.chdir(spec_kit_project) + lookup_id = "extension:missing:command:speckit.missing.command" + + result = CliRunner().invoke( + app, ["artifact", "lookup", lookup_id, "--json"] + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == { + "error": f"unknown contribution {lookup_id}" + } + + def test_lookup_requires_json_flag( + self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch + ): + monkeypatch.chdir(spec_kit_project) + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "extension:missing:command:speckit.missing.command", + ], + ) + + assert result.exit_code == 2 + assert result.stdout == "" + + def test_lookup_validates_project_before_lookup_id( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ): + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke( + app, + ["artifact", "lookup", "project:_:command:local", "--json"], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == { + "error": "not a Spec Kit project: no .specify/ directory found" + } + def test_hidden_command_layer_source_path_is_own_pack_file( self, spec_kit_project: Path ): @@ -1563,13 +1649,9 @@ def test_unregistered_preset_template_without_manifest(self, spec_kit_project: P row.id == "template:legacy-preset-template" for row in catalog.list_artifacts() ) info = catalog.get_artifact_info("legacy-preset-template") - resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( - "legacy-preset-template", "template" - )[0] assert info["stack"][0]["lookupId"] == ( "preset:legacy-preset:template:legacy-preset-template" ) - assert resolver_layer["lookupId"] == info["stack"][0]["lookupId"] def test_stale_registry_entry_with_missing_pack_dir_is_skipped( self, spec_kit_project: Path @@ -1822,6 +1904,18 @@ def test_declared_hook_has_artifact_and_stack_shape( "priority": 5, "optional": False, } + contribution = ArtifactCatalog(spec_kit_project).get_contribution_info( + entry["lookupId"] + ) + assert contribution["id"] == entry["lookupId"] + assert contribution["kind"] == "hook" + assert contribution["contribution"] == { + "eventName": "before_specify", + "command": "speckit.compliance.pre-check", + "description": "Compliance pre-check", + "priority": 5, + "optional": False, + } def test_duplicate_declarations_are_additive_and_priority_sorted( self, spec_kit_project: Path diff --git a/tests/test_artifact_command_parity.py b/tests/test_artifact_command_parity.py index 636c301a4f..18d96605ab 100644 --- a/tests/test_artifact_command_parity.py +++ b/tests/test_artifact_command_parity.py @@ -101,10 +101,15 @@ def test_preset_manifest_id_mismatch_uses_manifest_id(self, spec_kit_project: Pa assert active["lookupId"] == ( "preset:original-preset:command:speckit.preset-renamed.hello" ) + contribution = catalog.get_contribution_info(active["lookupId"]) + assert contribution["id"] == active["lookupId"] + assert contribution["layer"] == "preset" + assert contribution["sourceId"] == "original-preset" + assert contribution["contribution"]["file"] == "commands/actual.md" resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( "speckit.preset-renamed.hello", "command" )[0] - assert resolver_layer["lookupId"] == active["lookupId"] + assert "lookupId" not in resolver_layer # The stack row's presetId / manifestPath must still reflect the # actual on-disk directory (``renamed-preset``), not the manifest id # embedded in ``lookupId`` — otherwise the display and manifest path From 9eed32ebb91f0d52c987c545cf6fa67ed628859e Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 20:03:29 -0500 Subject: [PATCH 03/11] fix: preserve canonical hook event in lookup Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/artifacts/catalog.py | 2 +- tests/test_artifact_command.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index ecb6f9424f..a55533416b 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -507,7 +507,7 @@ def _find_extension_contribution( if relative_manifest is None: raise ArtifactResolutionError() return ( - {"eventName": event_name, **matching}, + {**matching, "eventName": event_name}, relative_manifest, None, ) diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index 2dee40e521..b0c206bf6a 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -1862,6 +1862,7 @@ def test_declared_hook_has_artifact_and_stack_shape( "before_specify": [ { "command": "speckit.compliance.pre-check", + "eventName": "after_plan", "description": "Compliance pre-check", "priority": 5, "optional": False, From 6c7e68d51f9276a1cf95514062c002b0b9296854 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 20:15:45 -0500 Subject: [PATCH 04/11] fix: continue duplicate hook provider lookup Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/artifacts/catalog.py | 2 +- tests/test_artifact_command.py | 44 ++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index a55533416b..11b216ec2a 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -500,7 +500,7 @@ def _find_extension_contribution( if isinstance(entry, dict) and entry.get("command") == command: matching = entry if matching is None: - return None + continue relative_manifest = _repo_relative_existing_file( self.project_root, manifest.path ) diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index b0c206bf6a..574f10be88 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -1741,6 +1741,7 @@ def _install_extension_with_hooks( extension_id: str, hooks: dict, *, + manifest_id: str | None = None, priority: int = 10, enabled: bool = True, ) -> Path: @@ -1750,8 +1751,8 @@ def _install_extension_with_hooks( manifest = { "schema_version": "1.0", "extension": { - "id": extension_id, - "name": extension_id, + "id": manifest_id or extension_id, + "name": manifest_id or extension_id, "version": "1.0.0", "description": "Test extension", "author": "test", @@ -1918,6 +1919,45 @@ def test_declared_hook_has_artifact_and_stack_shape( "optional": False, } + def test_hook_lookup_continues_past_same_id_manifest_without_target( + self, spec_kit_project: Path + ): + _install_extension_with_hooks( + spec_kit_project, + "a-copy", + manifest_id="shared-hooks", + hooks={"after_plan": [{"command": "unrelated.cmd"}]}, + ) + second = _install_extension_with_hooks( + spec_kit_project, + "b-copy", + manifest_id="shared-hooks", + hooks={ + "before_specify": [ + { + "command": "target.cmd", + "description": "Second manifest target", + } + ] + }, + ) + + catalog = ArtifactCatalog(spec_kit_project) + hook = next( + row + for row in catalog.list_artifacts_with_stack() + if row["kind"] == "hook" and row["targetCommand"] == "target.cmd" + ) + contribution = catalog.get_contribution_info( + hook["stack"][0]["lookupId"] + ) + + assert contribution["manifestPath"] == ( + second.relative_to(spec_kit_project).as_posix() + "/extension.yml" + ) + assert contribution["contribution"]["eventName"] == "before_specify" + assert contribution["contribution"]["command"] == "target.cmd" + def test_duplicate_declarations_are_additive_and_priority_sorted( self, spec_kit_project: Path ): From 74daf64de1e3d093a47f63e80945359582205609 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 20:17:08 -0500 Subject: [PATCH 05/11] test: cover encoded hook contribution lookup Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/test_artifact_command.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index 574f10be88..e51c33a337 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -2286,6 +2286,14 @@ def test_colon_containing_values_round_trip_through_encoded_id( info = ArtifactCatalog(spec_kit_project).get_artifact_info(row["id"]) assert info == row + contribution = ArtifactCatalog(spec_kit_project).get_contribution_info( + row["stack"][0]["lookupId"] + ) + assert contribution["contribution"]["eventName"] == "custom:after" + assert ( + contribution["contribution"]["command"] + == "/skill:speckit-test-ext-hello" + ) def test_kind_hint_resolves_hook_name(self, spec_kit_project: Path): _install_extension_with_hooks( From e5fb1a7b0b7f70f6d55ce7797629c04a13051bdb Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 20:44:12 -0500 Subject: [PATCH 06/11] fix: identify artifact lookups by installation Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/artifacts.md | 21 ++- src/specify_cli/artifacts/catalog.py | 16 +- src/specify_cli/artifacts/resolution.py | 20 +-- tests/test_artifact_command.py | 201 ++++++++++++++++++++++-- tests/test_artifact_command_parity.py | 69 ++++++-- 5 files changed, 281 insertions(+), 46 deletions(-) diff --git a/docs/reference/artifacts.md b/docs/reference/artifacts.md index 80604e368b..d544593ca6 100644 --- a/docs/reference/artifacts.md +++ b/docs/reference/artifacts.md @@ -135,7 +135,7 @@ For command, template, and script artifacts, `stack` is ordered by resolution pr | -------------- | -------------------------------------------------------------------------------- | | `id` | `{kind}:{name}` — the source-agnostic round-trip key, identical on every row of the same artifact's stack | | `layer` | `project`, `preset`, or `extension`; `null` for built-in layers | -| `sourceId` | Source component of `lookupId`, or `null` when the layer has no provenance | +| `sourceId` | Installed preset or extension ID used by the resolver, or `null` when the layer has no provenance | | `presetId` | Preset pack directory id; `null` on built-in, `project`, and `extension` rows | | `presetName` | Preset display name when its manifest declares one, else the pack id; `null` when `presetId` is `null` | | `strategy` | `replace`, `wrap`, `prepend`, `append`, or `additive` | @@ -147,7 +147,22 @@ For command, template, and script artifacts, `stack` is ordered by resolution pr `active` and `hidden` are independent labels, not opposites. For command, template, and script artifacts, `active` identifies the highest-precedence layer selected by the existing Spec Kit layer-resolution order; it does not validate that the layer content can be read or composed. This preserves the diagnostic behavior of `specify preset resolve`, which reports the discovered layer chain even when content composition later produces a warning. Composing strategies (`wrap`, `prepend`, `append`) keep lower layers in the composed output, so an inactive layer is not necessarily hidden: only layers below the first `replace` layer are marked `hidden`. Built-in rows have no provenance: `layer`, `sourceId`, and `lookupId` are `null` — but `id` is always populated, even on built-in rows. `id` is the round-trip key: `specify artifact info` accepts it as input (for example, `specify artifact info command:speckit.specify --json`), and it resolves the same artifact whether the caller passes the bare name or the `id`. -Lookup IDs are derived by the artifact command from the resolved layer and its existing preset or extension manifest. Manifest-declared layers use the manifest's `id`; convention-only layers use the installed preset or extension directory id. Project-local overrides carry a synthetic `project:_:{kind}:{name}` ID, while built-in layers have no `lookupId`. These values are artifact-stack provenance, not the round-trip key — use `id` for that. `sourcePath` is populated only when the layer maps to a concrete installed preset/extension file or a tracked agent materialization; core, project-override, and other synthetic rows report `null`. +Here, **provenance** means the origin of one artifact layer: the installed +preset or extension that supplied it, the manifest that declared it, and the +concrete file that backs it. Lookup IDs use the installed preset or extension +ID, matching the identity and ordering used by the existing resolver. The +installed ID is the registry/directory name and may differ from the logical +`id` declared inside the manifest. Using the installed ID keeps separate +installed directories distinct even when their manifests declare the same +logical ID. + +Project-local overrides carry a synthetic `project:_:{kind}:{name}` ID, while +built-in layers have no `lookupId`. These values identify artifact-stack +provenance; they are not the artifact round-trip key — use `id` with +`artifact info` for that. `manifestPath` identifies the declaring manifest, +and `sourcePath` identifies the concrete file backing the layer when one +exists. Core, project-override, and other synthetic rows may report +`sourcePath: null`. ## Contribution Lookup @@ -201,7 +216,7 @@ For hooks, `active` reports registration state from `.specify/extensions.yml`, n Declared-but-unregistered hooks remain visible when their extension is included by the normal resolver. Registry-disabled extensions are excluded entirely, consistently with their other contributions. Invalid individual extension manifests are also omitted by the existing resolver and remain diagnosable through extension inspection and validation commands. -Hook lookup IDs use the artifact-private `{layer}:{sourceId}:hook:{encodedEventName}:{encodedTargetCommand}` grammar. Hook provenance is restricted to `preset` and `extension` layers; hooks never receive a built-in/core layer. The current manifest API exposes extension hook declarations, so current rows use the `extension` layer. The `preset` layer remains reserved by the hook identifier grammar for preset-provided hooks without requiring artifact IDs to be added to preset or extension manifest APIs. +Hook lookup IDs use the artifact-private `{layer}:{sourceId}:hook:{encodedEventName}:{encodedTargetCommand}` grammar, where `sourceId` is the installed provider ID. Hook provenance is restricted to `preset` and `extension` layers; hooks never receive a built-in/core layer. The current manifest API exposes extension hook declarations, so current rows use the `extension` layer. The `preset` layer remains reserved by the hook identifier grammar for preset-provided hooks without requiring artifact IDs to be added to preset or extension manifest APIs. ## JSON Errors diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index 11b216ec2a..fa5d761be7 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -455,10 +455,12 @@ def _find_preset_contribution( name: str, ) -> tuple[dict[str, Any], str, str | None] | None: for pack_id, _metadata in resolver._get_all_presets_by_priority(): + if pack_id != source_id: + continue pack_dir = resolver.presets_dir / pack_id manifest = resolver._get_manifest(pack_dir) - if manifest is None or manifest.id != source_id: - continue + if manifest is None: + return None for entry in manifest.templates: if ( isinstance(entry, dict) @@ -484,14 +486,14 @@ def _find_extension_contribution( ) for _priority, extension_id, _metadata in resolver._get_all_extensions_by_priority(): + if extension_id != source_id: + continue extension_dir = resolver.extensions_dir / extension_id manifest_path = extension_dir / "extension.yml" try: manifest = ExtensionManifest(manifest_path) except (ValidationError, OSError, TypeError, AttributeError): - continue - if manifest.id != source_id: - continue + return None if kind == "hook": event_name, command = parse_hook_artifact_name(name) matching: dict[str, Any] | None = None @@ -500,7 +502,7 @@ def _find_extension_contribution( if isinstance(entry, dict) and entry.get("command") == command: matching = entry if matching is None: - continue + return None relative_manifest = _repo_relative_existing_file( self.project_root, manifest.path ) @@ -629,7 +631,7 @@ def _collect_hook_inventory( ) if manifest_path is None: raise ArtifactResolutionError() - source_id = manifest.id + source_id = extension_id for event_name, hook_config in (manifest.hooks or {}).items(): entries_by_command: dict[ diff --git a/src/specify_cli/artifacts/resolution.py b/src/specify_cli/artifacts/resolution.py index 73625a630f..707a1706bf 100644 --- a/src/specify_cli/artifacts/resolution.py +++ b/src/specify_cli/artifacts/resolution.py @@ -119,17 +119,9 @@ def _layer_provenance( declared = _manifest_entry_for_path( manifest, "extension", extension_dir, kind, name, path ) - source_id = ( - manifest.id - if declared is not None - and manifest is not None - and isinstance(manifest.id, str) - and manifest.id - else extension_id - ) return _LayerProvenance( "extension", - source_id, + extension_id, extension_id, extension_dir, manifest, @@ -148,17 +140,9 @@ def _layer_provenance( declared = _manifest_entry_for_path( manifest, "preset", pack_dir, kind, name, path ) - source_id = ( - manifest.id - if declared is not None - and manifest is not None - and isinstance(manifest.id, str) - and manifest.id - else pack_id - ) return _LayerProvenance( "preset", - source_id, + pack_id, pack_id, pack_dir, manifest, diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index e51c33a337..8d4727fec2 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -323,7 +323,9 @@ def test_excludes_disabled_and_unusable_manifest_contributions( assert "disabled-template" not in names assert "missing-template" not in names - def test_unregistered_extension_manifest_id_wins_for_lookup(self, spec_kit_project: Path): + def test_unregistered_extension_installed_id_identifies_lookup( + self, spec_kit_project: Path + ): ext_dir = spec_kit_project / ".specify" / "extensions" / "renamed" ext_dir.mkdir() (ext_dir / "commands").mkdir() @@ -368,16 +370,16 @@ def test_unregistered_extension_manifest_id_wins_for_lookup(self, spec_kit_proje row.id for row in catalog.list_artifacts() } info = catalog.get_artifact_info("speckit.original.hello") - # Artifact projection uses the manifest id without changing the - # resolver's established layer shape. - assert info["stack"][0]["lookupId"] == "extension:original:command:speckit.original.hello" + assert info["stack"][0]["sourceId"] == "renamed" + assert info["stack"][0]["lookupId"] == ( + "extension:renamed:command:speckit.original.hello" + ) resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( "speckit.original.hello", "command" )[0] assert "lookupId" not in resolver_layer - # The stack row's manifestPath must still reflect the actual on-disk - # extension directory (``renamed``), not the manifest id embedded in - # ``lookupId``. + # Provenance uses the installed directory identity and path even when + # the manifest declares a different logical ID. assert ( info["stack"][0]["manifestPath"] == ".specify/extensions/renamed/extension.yml" @@ -387,7 +389,7 @@ def test_unregistered_extension_manifest_id_wins_for_lookup(self, spec_kit_proje ) assert contribution["id"] == info["stack"][0]["lookupId"] assert contribution["layer"] == "extension" - assert contribution["sourceId"] == "original" + assert contribution["sourceId"] == "renamed" assert contribution["kind"] == "command" assert contribution["name"] == "speckit.original.hello" assert contribution["contribution"]["file"] == "commands/actual.md" @@ -403,6 +405,75 @@ def test_unregistered_extension_manifest_id_wins_for_lookup(self, spec_kit_proje with pytest.raises(ContributionNotFoundError): catalog.get_contribution_info(convention["lookupId"]) + def test_duplicate_extension_manifest_ids_resolve_each_installed_layer( + self, spec_kit_project: Path + ): + for installed_id, description in ( + ("a-copy", "First declaration"), + ("b-copy", "Second declaration"), + ): + ext_dir = ( + spec_kit_project / ".specify" / "extensions" / installed_id + ) + (ext_dir / "commands").mkdir(parents=True) + (ext_dir / "commands" / "shared.md").write_text( + description, encoding="utf-8" + ) + (ext_dir / "extension.yml").write_text( + yaml.safe_dump( + { + "schema_version": "1.0", + "extension": { + "id": "shared", + "name": "Shared", + "version": "1.0.0", + "description": "test", + "author": "test", + "repository": "https://example.com", + "license": "MIT", + }, + "requires": {"speckit_version": ">=0.2.0"}, + "provides": { + "commands": [ + { + "name": "speckit.shared.command", + "file": "commands/shared.md", + "description": description, + } + ] + }, + } + ), + encoding="utf-8", + ) + + catalog = ArtifactCatalog(spec_kit_project) + stack = catalog.get_artifact_info("speckit.shared.command")["stack"] + extension_layers = [ + layer for layer in stack if layer["layer"] == "extension" + ] + + assert [layer["sourceId"] for layer in extension_layers] == [ + "a-copy", + "b-copy", + ] + assert [layer["lookupId"] for layer in extension_layers] == [ + "extension:a-copy:command:speckit.shared.command", + "extension:b-copy:command:speckit.shared.command", + ] + resolved = [ + catalog.get_contribution_info(layer["lookupId"]) + for layer in extension_layers + ] + assert [ + contribution["contribution"]["description"] + for contribution in resolved + ] == ["First declaration", "Second declaration"] + assert [contribution["manifestPath"] for contribution in resolved] == [ + ".specify/extensions/a-copy/extension.yml", + ".specify/extensions/b-copy/extension.yml", + ] + def test_includes_project_local_core_assets(self, spec_kit_project: Path): templates_dir = spec_kit_project / ".specify" / "templates" (templates_dir / "legacy-template.md").write_text( @@ -922,6 +993,69 @@ def test_lookup_json_cross_references_manifest_contribution( payload = json.loads(result.stdout) assert payload["id"] == lookup_id assert payload["contribution"]["description"] == "Lookup target" + assert payload["sourcePath"] == ( + ".specify/presets/lookup-pack/templates/lookup.md" + ) + + def test_lookup_omits_missing_contribution_source( + self, spec_kit_project: Path + ): + install_preset( + spec_kit_project, + "missing-source", + { + "templates": [ + { + "type": "template", + "name": "missing-source", + "file": "templates/missing.md", + } + ] + }, + ) + + payload = ArtifactCatalog(spec_kit_project).get_contribution_info( + "preset:missing-source:template:missing-source" + ) + + assert payload["manifestPath"] == ( + ".specify/presets/missing-source/preset.yml" + ) + assert payload["sourcePath"] is None + + def test_lookup_omits_contribution_source_outside_provider( + self, spec_kit_project: Path, tmp_path: Path + ): + pack = install_preset( + spec_kit_project, + "escaping-source", + { + "templates": [ + { + "type": "template", + "name": "escaping-source", + "file": "templates/escape.md", + } + ] + }, + ) + outside = tmp_path / "outside.md" + outside.write_text("outside", encoding="utf-8") + link = pack / "templates" / "escape.md" + link.parent.mkdir() + try: + link.symlink_to(outside) + except OSError as exc: + pytest.skip(f"symlink creation unavailable: {exc}") + + payload = ArtifactCatalog(spec_kit_project).get_contribution_info( + "preset:escaping-source:template:escaping-source" + ) + + assert payload["manifestPath"] == ( + ".specify/presets/escaping-source/preset.yml" + ) + assert payload["sourcePath"] is None def test_lookup_json_rejects_unknown_contribution( self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch @@ -1919,7 +2053,7 @@ def test_declared_hook_has_artifact_and_stack_shape( "optional": False, } - def test_hook_lookup_continues_past_same_id_manifest_without_target( + def test_hook_lookup_targets_installed_provider_with_shared_manifest_id( self, spec_kit_project: Path ): _install_extension_with_hooks( @@ -1958,6 +2092,55 @@ def test_hook_lookup_continues_past_same_id_manifest_without_target( assert contribution["contribution"]["eventName"] == "before_specify" assert contribution["contribution"]["command"] == "target.cmd" + def test_same_manifest_id_hook_layers_have_distinct_lookup_ids( + self, spec_kit_project: Path + ): + for installed_id, description in ( + ("a-copy", "First hook"), + ("b-copy", "Second hook"), + ): + _install_extension_with_hooks( + spec_kit_project, + installed_id, + manifest_id="shared-hooks", + hooks={ + "before_specify": [ + { + "command": "target.cmd", + "description": description, + } + ] + }, + ) + + catalog = ArtifactCatalog(spec_kit_project) + hook = next( + row + for row in catalog.list_artifacts_with_stack() + if row["kind"] == "hook" and row["targetCommand"] == "target.cmd" + ) + + assert [entry["sourceId"] for entry in hook["stack"]] == [ + "a-copy", + "b-copy", + ] + assert [entry["lookupId"] for entry in hook["stack"]] == [ + "extension:a-copy:hook:before_specify:target.cmd", + "extension:b-copy:hook:before_specify:target.cmd", + ] + resolved = [ + catalog.get_contribution_info(entry["lookupId"]) + for entry in hook["stack"] + ] + assert [ + contribution["contribution"]["description"] + for contribution in resolved + ] == ["First hook", "Second hook"] + assert [contribution["manifestPath"] for contribution in resolved] == [ + ".specify/extensions/a-copy/extension.yml", + ".specify/extensions/b-copy/extension.yml", + ] + def test_duplicate_declarations_are_additive_and_priority_sorted( self, spec_kit_project: Path ): diff --git a/tests/test_artifact_command_parity.py b/tests/test_artifact_command_parity.py index 18d96605ab..cdd550eef9 100644 --- a/tests/test_artifact_command_parity.py +++ b/tests/test_artifact_command_parity.py @@ -62,7 +62,9 @@ def test_manifest_declared_artifact_matches_resolver(self, spec_kit_project: Pat assert active["layer"] == "preset" assert active["lookupId"] == "preset:test-manifest-parity:command:speckit.manifest-declared" - def test_preset_manifest_id_mismatch_uses_manifest_id(self, spec_kit_project: Path): + def test_preset_manifest_id_mismatch_uses_installed_id( + self, spec_kit_project: Path + ): pack = install_preset( spec_kit_project, "renamed-preset", @@ -96,27 +98,76 @@ def test_preset_manifest_id_mismatch_uses_manifest_id(self, spec_kit_project: Pa ) assert winner == "body-from-renamed-preset" - # Artifact projection uses the manifest's validated id regardless of - # the installed directory name. assert active["lookupId"] == ( - "preset:original-preset:command:speckit.preset-renamed.hello" + "preset:renamed-preset:command:speckit.preset-renamed.hello" ) contribution = catalog.get_contribution_info(active["lookupId"]) assert contribution["id"] == active["lookupId"] assert contribution["layer"] == "preset" - assert contribution["sourceId"] == "original-preset" + assert contribution["sourceId"] == "renamed-preset" assert contribution["contribution"]["file"] == "commands/actual.md" resolver_layer = PresetResolver(spec_kit_project).collect_all_layers( "speckit.preset-renamed.hello", "command" )[0] assert "lookupId" not in resolver_layer - # The stack row's presetId / manifestPath must still reflect the - # actual on-disk directory (``renamed-preset``), not the manifest id - # embedded in ``lookupId`` — otherwise the display and manifest path - # would point to a non-existent location. + assert active["sourceId"] == "renamed-preset" assert active["presetId"] == "renamed-preset" assert active["manifestPath"] == ".specify/presets/renamed-preset/preset.yml" + def test_duplicate_preset_manifest_ids_resolve_each_installed_layer( + self, spec_kit_project: Path + ): + for installed_id, description in ( + ("a-copy", "First declaration"), + ("b-copy", "Second declaration"), + ): + pack = install_preset( + spec_kit_project, + installed_id, + { + "commands": [ + { + "name": "speckit.shared.command", + "file": "commands/shared.md", + "description": description, + } + ] + }, + ) + manifest_path = pack / "preset.yml" + manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + manifest["preset"]["id"] = "shared" + manifest_path.write_text(yaml.safe_dump(manifest), encoding="utf-8") + (pack / "commands").mkdir() + (pack / "commands" / "shared.md").write_text( + description, encoding="utf-8" + ) + + catalog = ArtifactCatalog(spec_kit_project) + stack = catalog.get_artifact_info("speckit.shared.command")["stack"] + preset_layers = [layer for layer in stack if layer["layer"] == "preset"] + + assert [layer["sourceId"] for layer in preset_layers] == [ + "a-copy", + "b-copy", + ] + assert [layer["lookupId"] for layer in preset_layers] == [ + "preset:a-copy:command:speckit.shared.command", + "preset:b-copy:command:speckit.shared.command", + ] + resolved = [ + catalog.get_contribution_info(layer["lookupId"]) + for layer in preset_layers + ] + assert [ + contribution["contribution"]["description"] + for contribution in resolved + ] == ["First declaration", "Second declaration"] + assert [contribution["manifestPath"] for contribution in resolved] == [ + ".specify/presets/a-copy/preset.yml", + ".specify/presets/b-copy/preset.yml", + ] + def test_module_imports(): _ = ArtifactCatalog From d3da088c9fc827be671a41962682fd40ace73a2d Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 20:54:05 -0500 Subject: [PATCH 07/11] fix: reject non-json artifact contributions Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/artifacts/catalog.py | 5 +++ tests/test_artifact_command.py | 62 ++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index fa5d761be7..640db9ba9d 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -11,6 +11,7 @@ from __future__ import annotations +import json import re import shlex from collections.abc import Iterable @@ -436,6 +437,10 @@ def get_contribution_info(self, lookup_id: str) -> dict[str, Any]: raise ContributionNotFoundError(lookup_id) contribution, manifest_path, source_path = resolved + try: + json.dumps(contribution) + except (TypeError, ValueError) as exc: + raise ArtifactResolutionError() from exc return { "id": lookup_id, "layer": layer, diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index 8d4727fec2..816c376a6d 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -11,6 +11,7 @@ import os import re import shutil +from datetime import date from pathlib import Path import pytest @@ -1073,6 +1074,67 @@ def test_lookup_json_rejects_unknown_contribution( "error": f"unknown contribution {lookup_id}" } + def test_lookup_json_rejects_non_json_manifest_value( + self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch + ): + monkeypatch.chdir(spec_kit_project) + install_preset( + spec_kit_project, + "dated-contribution", + { + "templates": [ + { + "type": "template", + "name": "dated-contribution", + "released": date(2026, 1, 1), + } + ] + }, + ) + + result = CliRunner().invoke( + app, + [ + "artifact", + "lookup", + "preset:dated-contribution:template:dated-contribution", + "--json", + ], + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == { + "error": "artifact resolution failed" + } + + @pytest.mark.parametrize( + "lookup_id", + [ + "invalid:source:command:name", + "extension:source:invalid:name", + "extension:source:hook:%FF:command", + "extension:source:hook:event:%ZZ", + ], + ) + def test_lookup_json_rejects_malformed_lookup_id( + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + lookup_id: str, + ): + monkeypatch.chdir(spec_kit_project) + + result = CliRunner().invoke( + app, ["artifact", "lookup", lookup_id, "--json"] + ) + + assert result.exit_code == 1 + assert result.stdout == "" + assert json.loads(result.stderr) == { + "error": f"unknown contribution {lookup_id}" + } + def test_lookup_requires_json_flag( self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch ): From 151d354e6ce208cd3d1e4307ff6897aace2862f1 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 21:00:50 -0500 Subject: [PATCH 08/11] fix: enforce strict artifact json Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/artifacts/catalog.py | 2 +- tests/test_artifact_command.py | 23 ++++++++++++++++++----- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index 640db9ba9d..c91101c3c8 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -438,7 +438,7 @@ def get_contribution_info(self, lookup_id: str) -> dict[str, Any]: contribution, manifest_path, source_path = resolved try: - json.dumps(contribution) + json.dumps(contribution, allow_nan=False) except (TypeError, ValueError) as exc: raise ArtifactResolutionError() from exc return { diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index 816c376a6d..d69d7972d7 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -1074,19 +1074,32 @@ def test_lookup_json_rejects_unknown_contribution( "error": f"unknown contribution {lookup_id}" } + @pytest.mark.parametrize( + "manifest_value", + [ + date(2026, 1, 1), + float("nan"), + float("inf"), + float("-inf"), + ], + ids=["date", "nan", "positive-infinity", "negative-infinity"], + ) def test_lookup_json_rejects_non_json_manifest_value( - self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch + self, + spec_kit_project: Path, + monkeypatch: pytest.MonkeyPatch, + manifest_value: object, ): monkeypatch.chdir(spec_kit_project) install_preset( spec_kit_project, - "dated-contribution", + "non-json-contribution", { "templates": [ { "type": "template", - "name": "dated-contribution", - "released": date(2026, 1, 1), + "name": "non-json-contribution", + "extra": manifest_value, } ] }, @@ -1097,7 +1110,7 @@ def test_lookup_json_rejects_non_json_manifest_value( [ "artifact", "lookup", - "preset:dated-contribution:template:dated-contribution", + "preset:non-json-contribution:template:non-json-contribution", "--json", ], ) From a469f97fb0a8cfebb0bb380284c4386f6177ede7 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 21:15:49 -0500 Subject: [PATCH 09/11] fix: align lookup with manifest semantics Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/reference/artifacts.md | 21 +++-- src/specify_cli/artifacts/_commands.py | 4 +- src/specify_cli/artifacts/catalog.py | 6 +- tests/test_artifact_command.py | 105 +++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 12 deletions(-) diff --git a/docs/reference/artifacts.md b/docs/reference/artifacts.md index d544593ca6..35bbec9f4f 100644 --- a/docs/reference/artifacts.md +++ b/docs/reference/artifacts.md @@ -170,16 +170,21 @@ exists. Core, project-override, and other synthetic rows may report specify artifact lookup --json ``` -Resolves a manifest-backed stack `lookupId` to the exact preset or extension -declaration that produced it. This keeps cross-reference behavior inside the -artifact command: preset and extension commands, manifests, and resolver return -shapes are unchanged. +Resolves a manifest-backed stack `lookupId` to the validated preset or extension +declaration that Spec Kit uses. The returned declaration reflects normal +manifest processing, including canonical extension command names, injected +defaults such as empty alias lists, and lowercase preset strategies. It is not +a verbatim representation of the authored YAML. + +This keeps cross-reference behavior inside the artifact command: preset and +extension commands, manifests, and resolver return shapes are unchanged. The response includes the stable lookup ID, provider coordinates, manifest and -source paths, and the original declaration under `contribution`. Convention-only -contributions and project overrides have no originating manifest declaration, -so lookup returns `{"error": "unknown contribution "}` with exit code -`1`. Built-in rows never have a `lookupId`. +source paths, and the effective declaration under `contribution`. +Convention-only contributions and project overrides have no originating +manifest declaration, so lookup returns +`{"error": "unknown contribution "}` with exit code `1`. Built-in +rows never have a `lookupId`. ### Hook artifacts diff --git a/src/specify_cli/artifacts/_commands.py b/src/specify_cli/artifacts/_commands.py index 66bea8e18a..49a991f0f7 100644 --- a/src/specify_cli/artifacts/_commands.py +++ b/src/specify_cli/artifacts/_commands.py @@ -164,10 +164,10 @@ def artifact_lookup( json_flag: bool = typer.Option( False, "--json", - help="Emit the originating manifest contribution as JSON.", + help="Emit the validated manifest contribution used by Spec Kit as JSON.", ), ) -> None: - """Resolve a stack lookupId to its preset or extension contribution.""" + """Resolve a stack lookupId to its effective preset or extension contribution.""" _require_json_flag(json_flag) try: root = _resolve_project_root() diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index c91101c3c8..99909b8e4a 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -409,7 +409,7 @@ def get_artifact_info( } def get_contribution_info(self, lookup_id: str) -> dict[str, Any]: - """Resolve a stack ``lookupId`` to its originating manifest entry.""" + """Resolve a stack ``lookupId`` to its validated manifest entry.""" _validate_project(self.project_root) _validate_extension_registry(self.project_root) try: @@ -667,6 +667,7 @@ def _collect_hook_inventory( "id": public_id, "layer": "extension", "sourceId": source_id, + "runtimeExtensionId": manifest.id, "presetId": None, "presetName": None, "manifestPath": manifest_path, @@ -709,7 +710,8 @@ def _collect_hook_inventory( presetName=None, strategy="additive", active=any( - binding.get("extension") == declaration["sourceId"] + binding.get("extension") + == declaration["runtimeExtensionId"] and binding.get("command") == command for binding in enabled_bindings ), diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index d69d7972d7..a938619374 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -998,6 +998,74 @@ def test_lookup_json_cross_references_manifest_contribution( ".specify/presets/lookup-pack/templates/lookup.md" ) + def test_lookup_returns_normalized_preset_declaration( + self, spec_kit_project: Path + ): + install_preset( + spec_kit_project, + "normalized-preset", + { + "templates": [ + { + "type": "template", + "name": "normalized-template", + "strategy": "APPEND", + } + ] + }, + ) + + payload = ArtifactCatalog(spec_kit_project).get_contribution_info( + "preset:normalized-preset:template:normalized-template" + ) + + assert payload["contribution"]["strategy"] == "append" + + def test_lookup_returns_normalized_extension_declaration( + self, spec_kit_project: Path + ): + extension_dir = ( + spec_kit_project / ".specify" / "extensions" / "normalized-extension" + ) + extension_dir.mkdir() + (extension_dir / "commands").mkdir() + (extension_dir / "commands" / "hello.md").write_text( + "body", encoding="utf-8" + ) + (extension_dir / "extension.yml").write_text( + yaml.safe_dump( + { + "schema_version": "1.0", + "extension": { + "id": "normalized-extension", + "name": "Normalized extension", + "version": "1.0.0", + "description": "test", + }, + "requires": {"speckit_version": ">=0.2.0"}, + "provides": { + "commands": [ + { + "name": "speckit.hello", + "file": "commands/hello.md", + } + ] + }, + } + ), + encoding="utf-8", + ) + + payload = ArtifactCatalog(spec_kit_project).get_contribution_info( + "extension:normalized-extension:command:" + "speckit.normalized-extension.hello" + ) + + assert payload["contribution"]["name"] == ( + "speckit.normalized-extension.hello" + ) + assert payload["contribution"]["aliases"] == [] + def test_lookup_omits_missing_contribution_source( self, spec_kit_project: Path ): @@ -2476,6 +2544,43 @@ def test_duplicate_contributors_activate_independently( assert active_by_source == {"ext-a": False, "ext-b": True} assert row["registered"] is True + def test_renamed_installation_uses_manifest_id_for_runtime_activation( + self, spec_kit_project: Path + ): + _install_extension_with_hooks( + spec_kit_project, + "renamed-installation", + manifest_id="runtime-id", + hooks={ + "before_specify": [ + {"command": "speckit.runtime-id.pre-check"} + ] + }, + ) + _write_hook_binding( + spec_kit_project, + "before_specify", + [ + { + "extension": "runtime-id", + "command": "speckit.runtime-id.pre-check", + "enabled": True, + } + ], + ) + + rows = ArtifactCatalog(spec_kit_project).list_artifacts_with_stack() + row = next(item for item in rows if item["kind"] == "hook") + entry = row["stack"][0] + + assert entry["sourceId"] == "renamed-installation" + assert entry["lookupId"] == ( + "extension:renamed-installation:hook:" + "before_specify:speckit.runtime-id.pre-check" + ) + assert entry["active"] is True + assert row["registered"] is True + def test_invalid_runtime_config_degrades_to_unregistered( self, spec_kit_project: Path ): From 7b9ca4c1d478682e3af4f66b652923799ddcf168 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 21:18:49 -0500 Subject: [PATCH 10/11] fix: preserve lexical artifact source paths Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/artifacts/catalog.py | 4 ++-- tests/test_artifact_command.py | 33 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index 99909b8e4a..8aa98f74ef 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -547,8 +547,8 @@ def _contribution_result( if isinstance(relative_file, str): try: resolved_pack = pack_dir.resolve() - candidate = (pack_dir / relative_file).resolve() - candidate.relative_to(resolved_pack) + candidate = pack_dir / relative_file + candidate.resolve().relative_to(resolved_pack) except (OSError, ValueError): pass else: diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index a938619374..ea302e1c96 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -1126,6 +1126,39 @@ def test_lookup_omits_contribution_source_outside_provider( ) assert payload["sourcePath"] is None + def test_lookup_preserves_source_path_through_symlinked_project_root( + self, spec_kit_project: Path, tmp_path: Path + ): + pack = install_preset( + spec_kit_project, + "symlinked-project", + { + "templates": [ + { + "type": "template", + "name": "symlinked-project", + "file": "templates/source.md", + } + ] + }, + ) + source = pack / "templates" / "source.md" + source.parent.mkdir() + source.write_text("body", encoding="utf-8") + linked_project = tmp_path / "linked-project" + try: + linked_project.symlink_to(spec_kit_project, target_is_directory=True) + except OSError as exc: + pytest.skip(f"directory symlink creation unavailable: {exc}") + + payload = ArtifactCatalog(linked_project).get_contribution_info( + "preset:symlinked-project:template:symlinked-project" + ) + + assert payload["sourcePath"] == ( + ".specify/presets/symlinked-project/templates/source.md" + ) + def test_lookup_json_rejects_unknown_contribution( self, spec_kit_project: Path, monkeypatch: pytest.MonkeyPatch ): From 4b55a5c8e79796c29e948de925639c19ce2fe733 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Fri, 11 Sep 2026 21:27:18 -0500 Subject: [PATCH 11/11] fix: validate artifact lookup output encoding Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/artifacts/_commands.py | 15 ++++++++++++++- src/specify_cli/artifacts/catalog.py | 5 ----- tests/test_artifact_command.py | 9 ++++++++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/artifacts/_commands.py b/src/specify_cli/artifacts/_commands.py index 49a991f0f7..2c129df03e 100644 --- a/src/specify_cli/artifacts/_commands.py +++ b/src/specify_cli/artifacts/_commands.py @@ -179,7 +179,20 @@ def artifact_lookup( _emit_error_and_exit(ArtifactResolutionError()) return # pragma: no cover - sys.stdout.write(json.dumps(payload, indent=2, sort_keys=True, ensure_ascii=False)) + try: + rendered = json.dumps( + payload, + indent=2, + sort_keys=True, + ensure_ascii=False, + allow_nan=False, + ) + rendered.encode("utf-8") + except (TypeError, ValueError, UnicodeEncodeError): + _emit_error_and_exit(ArtifactResolutionError()) + return # pragma: no cover + + sys.stdout.write(rendered) sys.stdout.write("\n") diff --git a/src/specify_cli/artifacts/catalog.py b/src/specify_cli/artifacts/catalog.py index 8aa98f74ef..d56a3b0a59 100644 --- a/src/specify_cli/artifacts/catalog.py +++ b/src/specify_cli/artifacts/catalog.py @@ -11,7 +11,6 @@ from __future__ import annotations -import json import re import shlex from collections.abc import Iterable @@ -437,10 +436,6 @@ def get_contribution_info(self, lookup_id: str) -> dict[str, Any]: raise ContributionNotFoundError(lookup_id) contribution, manifest_path, source_path = resolved - try: - json.dumps(contribution, allow_nan=False) - except (TypeError, ValueError) as exc: - raise ArtifactResolutionError() from exc return { "id": lookup_id, "layer": layer, diff --git a/tests/test_artifact_command.py b/tests/test_artifact_command.py index ea302e1c96..b574dad7b9 100644 --- a/tests/test_artifact_command.py +++ b/tests/test_artifact_command.py @@ -1182,8 +1182,15 @@ def test_lookup_json_rejects_unknown_contribution( float("nan"), float("inf"), float("-inf"), + "\ud800", + ], + ids=[ + "date", + "nan", + "positive-infinity", + "negative-infinity", + "unpaired-surrogate", ], - ids=["date", "nan", "positive-infinity", "negative-infinity"], ) def test_lookup_json_rejects_non_json_manifest_value( self,