feat: add deterministic contribution IDs and specify artifact command

[nicolehaugen]

Consolidates the work from #4261 and #4267 into a single PR against main.

Why consolidated

#4261 (contribution IDs) and #4267 (specify artifact command) were originally opened as a stack, but the underlying identifier grammar and resolver changes evolved together through review — splitting them cleanly would leave both PRs with incomplete stories. This PR ships the full, cohesive story in one place.

What's in it

Contribution IDs (from #4261):

• Deterministic {layer}:{source_id}:{kind}:{name} identifier grammar for every core / preset / extension / project-override contribution.
• PresetResolver.collect_all_layers() returns each layer with a stable lookupId so the same ID round-trips through the resolver.
• _identifier.py module owns the grammar; consumers never re-derive it via string prefixes.

Artifact command (from #4267):

• New specify artifact command that surfaces composition stacks as JSON, keyed by the same lookup IDs above.
• _script_variants.py module: single place that knows the bash/, powershell/, python/ runtime subdir layout for core scripts.
• Shared _locate_core_asset_dir in _assets.py — one source of truth for locating core templates/, commands/, and scripts/ across wheel-installed and source-checkout layouts.

Fixes surfaced during review:

• collect_all_layers() uses the on-disk directory name as the extension sourceId, matching how iter_extensions_by_priority() and the artifact catalog enumerate extensions — closes a gap where a manifest with a declared id different from its directory produced a lookupId the resolver would not itself hand back.
• Preset-manifest projection branches on manifest kind rather than folding all preset contributions into provides.templates, so an installed override like speckit.constitution no longer becomes an ambiguous template: row.
• Project-override layer is serialized with its own project: layer prefix instead of being labeled as a preset with sourceId _.

Fixes

Fixes #4210
Fixes #4212

Supersedes

Supersedes #4261 (closed)
Supersedes #4267 (closed)

Testing

1132 tests passing on Windows (11 skipped, 127 deselected are pre-existing Windows symlink-privilege failures unrelated to these changes, confirmed by running the same tests against unmodified origin/main).

---

Posted on behalf of @nicolehaugen by GitHub Copilot (model: claude-opus-4.7, supervised).

[Copilot]

Review details

Suppressed comments (4)

src/specify_cli/artifacts/init.py:587

• A replace layer does not guarantee resolvable content. For example, the existing non-UTF-8 legacy-layer case makes resolve_content() return None, but this condition still adds the artifact to list; its advertised ID then fails with unknown artifact in info. Gate inventory membership on successful composition so every listed row is retrievable.

        bare, resolved_kind = _resolve_kind_hint(name, kind)

        from ..presets import PresetError, PresetResolver  # lazy: avoids circular import

src/specify_cli/artifacts/init.py:634

• get_artifact_info() rebuilds the complete inventory before resolving one artifact. Since list_artifacts() scans every candidate across every preset/extension, the documented consumer pattern of calling info for each listed artifact becomes quadratic in the artifact count. Resolve the requested kind/name directly (checking at most the three kinds only for an unqualified name) and build just that stack.

            "stack": [layer.to_json_dict() for layer in stack],

src/specify_cli/_identifier.py:149

• This parser accepts IDs forbidden by the declared grammar, including core:x:command:y, project:x:template:y, and core:_:hook:e:c. Downstream callers treat a returned layer as authoritative. Enforce the _ source sentinel for core/project and restrict hooks to preset/extension; apply the same cross-component rules in the derivation helpers so construction and parsing remain consistent.

    if parts[2] not in _CONTRIBUTION_KINDS:
        return None
    expected_len = 5 if parts[2] == "hook" else 4
    if len(parts) != expected_len:
        return None

src/specify_cli/presets/init.py:5735

• The current PR description says this sourceId is the on-disk extension directory name, but this branch deliberately substitutes the validated manifest ID and the tests assert that behavior. The implementation matches the round-trip requirement; update the PR description to state that directory identity is only the convention-only fallback.

                            ext_manifest = ExtensionManifest(ext_manifest_path)
                            if isinstance(ext_manifest.id, str) and ext_manifest.id:
                                source_id_for_lookup = ext_manifest.id

• Files reviewed: 22/22 changed files
• Comments generated: 3
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (3)

src/specify_cli/artifacts/init.py:250

• The public adapter discards a core layer’s identity even though the linked #4212 contract requires baseline rows to expose layer: "core" and lookupId: "core:_:{kind}:{name}". The resolver already supplies that exact ID, so converting it to three null values prevents consumers from joining or diagnosing the core contribution. Preserve the core identity here and update the public layer type, tests, and artifact reference accordingly.

    if (
        resolver_layer.get("source") == "core"
        and isinstance(lookup_id, str)
        and lookup_id.startswith("core:_:")
    ):
        return None, None, None

docs/reference/presets.md:223

• This states that built-in resolver layers omit lookupId, but collect_all_layers() now explicitly emits core:_:{kind}:{name} for both project-local and bundled core layers (see src/specify_cli/presets/__init__.py:5782-5803), and the new resolver tests assert it. Document that core lookup ID instead; only the artifact-output adapter currently removes it.

`PresetResolver.collect_all_layers()` returns layer dicts that include a `lookupId` field for preset, extension, and project-override layers. For manifest-declared preset and extension layers, the `lookupId`'s `sourceId` component is the manifest's validated `id:` field, so it joins directly to the `id` used by `PresetManifest.iter_contributions()` / `ExtensionManifest.iter_contributions()` even when the installed directory was renamed. That join is guaranteed by the implementation, so consumers can key off `lookupId` directly rather than re-deriving the contribution id. Convention-only layers (undeclared in any manifest) have no manifest id to consult, so their `lookupId`'s `sourceId` falls back to the resolver's registry key or on-disk directory name instead; those layers have no manifest contribution to join to. Built-in fallback layers omit `lookupId`. Use `layer_kind_from_lookup_id` to classify lookup IDs rather than parsing the string yourself. 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.

extensions/EXTENSION-API-REFERENCE.md:903

• The built-in-layer sentence contradicts the API it documents: PresetResolver.collect_all_layers() now returns core:_:{kind}:{name} on every core layer, with regression assertions in tests/test_presets.py. Clarify that resolver behavior rather than saying the field is omitted.

`PresetResolver.collect_all_layers()` returns layer dicts that include a `lookupId` field for project overrides, preset contributions, and extension contributions. Manifest-declared preset and extension layers use the manifest's validated `id:` as the `lookupId` source id, so it matches the id `iter_contributions()` yields for that same contribution. Convention-only layers (no manifest entry declares the contribution) have no manifest id to consult, so their `lookupId` falls back to the resolver's registry key or on-disk directory name. Built-in fallback layers omit `lookupId`.

• Files reviewed: 22/22 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

src/specify_cli/artifacts/init.py:1004

• A stale registry entry whose preset/extension directory was deleted reaches this unconditional iterdir() and turns the entire artifact inventory into ArtifactResolutionError, while PresetResolver simply skips the missing pack. Return without yielding when pack_dir is not a directory so artifact enumeration remains aligned with resolution.

    for entry in sorted(pack_dir.iterdir(), key=lambda p: p.name):

src/specify_cli/_identifier.py:104

• This constructor validates components independently but not their relationship, so it can emit IDs outside the documented grammar, such as project:foo:template:x or preset:_:template:x. Enforce that _ is used exactly for the project layer so every constructed ID remains parseable under the advertised contract.

    if layer not in _LAYER_KINDS:
        raise IdentifierComponentError(f"Invalid layer '{layer}'")
    if kind not in _NAMED_CONTRIBUTION_KINDS:
        raise IdentifierComponentError(f"Invalid named contribution kind '{kind}'")

src/specify_cli/_identifier.py:145

• The complete-shape parser still accepts combinations that the identifier grammar and constructors forbid: project:_:hook:event:command, project:foo:template:x, and preset:_:template:x are all classified as valid layers. Reject project hooks and enforce the project/_ source relationship before callers treat malformed IDs as authoritative.

    layer = parts[0]
    if layer not in _LAYER_KINDS:
        return None
    if parts[2] not in _CONTRIBUTION_KINDS:
        return None
    expected_len = 5 if parts[2] == "hook" else 4

src/specify_cli/presets/init.py:5797

• The PR description says collect_all_layers() gives each layer a lookupId, but the intended built-in contract implemented here and in the reference docs deliberately omits it. Update the PR description to state that built-in layers are recognized by the absence of lookupId, layer, and sourceId.

                layers.append({
                    "path": bundled,
                    "source": "core",
                    "strategy": "replace",
                })

src/specify_cli/presets/init.py:5735

• The current PR description says extension sourceId comes from the on-disk directory, but this branch intentionally emits the manifest-declared ID and reserves the directory name for convention-only contributions. Update the PR description so it does not publish the opposite lookup contract.

                            ext_manifest = ExtensionManifest(ext_manifest_path)
                            if isinstance(ext_manifest.id, str) and ext_manifest.id:
                                source_id_for_lookup = ext_manifest.id

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

src/specify_cli/artifacts/init.py:304

• Convention-only preset/extension layers can still carry pack_dir/extension_dir while a valid pack manifest exists but does not declare this artifact. This function then publishes that file as manifestPath, even though the lookup ID cannot be found in it, contradicting the field's “declaring manifest” contract. Carry whether the resolver match was manifest-declared and return None here for convention-only layers.

    if isinstance(pack_dir, Path):
        manifest_path = pack_dir / manifest_name
    elif pack_id:
        manifest_path = project_root / ".specify" / tier_dir / pack_id / manifest_name
    else:
        return None
    if not manifest_path.is_file():
        return None

src/specify_cli/_identifier.py:49

• This “every layer carries a lookupId” statement contradicts the module contract above and the public artifact schema: built-in layers intentionally omit lookupId. Describe this as a synthetic identifier specific to project-override layers so consumers do not infer that built-in layers also have one.

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.

src/specify_cli/presets/init.py:5735

• The current PR description still says extension sourceId comes from the on-disk directory, but manifest-declared layers here deliberately use ExtensionManifest.id; the tests and reference docs also enforce that direct join. Update the PR description to reserve directory identity for convention-only contributions.

                            ext_manifest = ExtensionManifest(ext_manifest_path)
                            if isinstance(ext_manifest.id, str) and ext_manifest.id:
                                source_id_for_lookup = ext_manifest.id

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

src/specify_cli/_identifier.py:106

• derive_named_id() still accepts a project layer with any non-empty source ID (for example, project:custom:template:x), even though this module’s grammar fixes project overrides to project:_:{kind}:{name}. The parser likewise classifies that impossible value as valid, so the shared constructor/parser can publish IDs outside the documented contract. Enforce _ for project source IDs in both construction and parsing.

    if layer not in _LAYER_KINDS:
        raise IdentifierComponentError(f"Invalid layer '{layer}'")
    if kind not in _NAMED_CONTRIBUTION_KINDS:
        raise IdentifierComponentError(f"Invalid named contribution kind '{kind}'")
    validate_component(name, "name")

src/specify_cli/artifacts/init.py:626

• At this point the artifact was already found in the inventory, so resolve_content() is None means its content could not be composed (for example, an existing replace-layer file is not valid UTF-8), not that the artifact is unknown. Returning ArtifactNotFoundError makes a listed ID fail with the wrong error and contradicts the documented artifact resolution failed case for composition failures.

            if PresetResolver(self.project_root).resolve_content(
                validated_name, resolved_kind
            ) is None:
                raise ArtifactNotFoundError(name)

src/specify_cli/presets/init.py:5735

• The current PR description says collect_all_layers() uses the on-disk extension directory as sourceId, but this code intentionally uses the validated manifest ID for declared entries and reserves the directory name for convention-only fallback. Update the PR description to state the implemented contract; otherwise consumers are told the opposite lookup-ID identity rule.

                            ext_manifest = ExtensionManifest(ext_manifest_path)
                            if isinstance(ext_manifest.id, str) and ext_manifest.id:
                                source_id_for_lookup = ext_manifest.id

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[nicolehaugen]

@copilot Align the new artifact command with pre-existing Spec Kit command conventions. Make these three changes:

1. Reuse the shared project-detection helper. In src/specify_cli/artifacts/_commands.py, delete the module-local _resolve_project_root() and use _require_specify_project() from src/specify_cli/__init__.py:532 (which every preset command already uses). If the JSON-envelope error contract requires suppressing the Rich console output, wrap the shared helper — do not duplicate its logic. Remove NotASpecKitProjectError from artifacts/__init__.py if it becomes unused, or keep it only as a thin adapter that catches typer.Exit from the shared helper and re-emits the JSON envelope.
2. Rename CLI handler functions to match the <subapp>_<verb> convention used by every preset command (preset_list, preset_add, preset_resolve, ...). Rename list_command → artifact_list and info_command → artifact_info in src/specify_cli/artifacts/_commands.py. Update any test references.
3. Apply registry validation consistently. In src/specify_cli/artifacts/__init__.py, list_artifacts() (around line 552) currently skips _validate_extension_registry() / _validate_preset_registry(), but get_artifact_info() calls both. Either call the validators in list_artifacts() too, or add an inline comment explaining the intentional asymmetry. The is_corrupt() fail-closed pattern must be uniform across both commands.

Do not change the JSON field naming (camelCase in stack rows comes directly from PresetResolver.collect_all_layers() and should stay). Do not change the docs' error-message table or exit-code split — those are correct.

[Copilot]

@copilot Align the new artifact command with pre-existing Spec Kit command conventions. Make these three changes:

1. Reuse the shared project-detection helper. In `src/specify_cli/artifacts/_co...

Addressed in 1c6bb8b:

1. _resolve_project_root() now delegates to _require_specify_project(), redirecting stderr during the call and converting its typer.Exit into NotASpecKitProjectError for the JSON envelope. NotASpecKitProjectError stays as that thin adapter.
2. Renamed list_command/info_command → artifact_list/artifact_info. No tests referenced the old function names directly.
3. Removed the redundant direct validator calls in get_artifact_info() — both it and list_artifacts() now validate exactly once, via _collect_inventory().

85 artifact tests + 762 tests across artifacts/presets/contribution-ids pass, and ruff is clean.

[Copilot]

Review details

Suppressed comments (3)

docs/reference/artifacts.md:131

• This cause does not match the implementation: composition returning None is currently emitted as unknown artifact, not artifact resolution failed, while invalid manifests are intentionally ignored in favor of convention discovery. Document the actual resolution-error cases instead.

| `artifact resolution failed`                        | The preset/extension registries could not be read, or artifact content could not be composed |

src/specify_cli/presets/init.py:5735

• The PR description says extension sourceId uses the on-disk directory name, but this branch deliberately uses ExtensionManifest.id for manifest-declared contributions (and the tests/docs assert that contract). The implementation supports the required lookupId round-trip, so update the PR description to reserve directory identity for convention-only contributions.

                            ext_manifest = ExtensionManifest(ext_manifest_path)
                            if isinstance(ext_manifest.id, str) and ext_manifest.id:
                                source_id_for_lookup = ext_manifest.id

src/specify_cli/_identifier.py:49

• “Every layer carries a lookupId” contradicts this module's built-in-tier contract: core layers intentionally omit lookupId. Narrow this statement to project-override layers so consumers are not told that the invariant applies to core.

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.

• Files reviewed: 22/22 changed files
• Comments generated: 1
• Review effort level: Balanced

[nicolehaugen]

@copilot

Rename `PresetResolver.core_name_candidates` to be source-agnostic and update its docstring, since the method's behavior is not core-specific — it applies the Spec Kit-wide `speckit.` filename convention (see the existing `_core_stem` helper and its uses at `src/specify_cli/presets/__init__.py:194`, `:2334`, `:2590`, `:5411-5419`, `:5765-5774`).

Scope (limited to PR-added code — do NOT modify the pre-existing resolve sites or `_core_stem`):

1. In `src/specify_cli/presets/__init__.py` (around line 5294-5301), rename the classmethod:

From: `core_name_candidates`To: `name_candidates`

Rewrite the docstring to describe the convention without naming a tier. Suggested wording:

Return exact-first filename candidates for a speckit.<stem> logical name.

Given a logical name like speckit.plan, returns ["speckit.plan", "plan"]

so callers can try the fully-qualified filename first and fall back to the

bare stem. Names that do not follow the speckit.<stem> convention return

a single-element list containing the original name.

[Copilot]

@copilot

Rename `PresetResolver.core_name_candidates` to be source-agnostic and update its docstring, since the method's behavior is not core-specific — it applies the Spec Kit-wide `speckit....

Addressed in bd483da: renamed PresetResolver.core_name_candidates to PresetResolver.name_candidates and updated the docstring to source-agnostic speckit.<stem> convention wording, with call-site usage updated accordingly.

Posted on behalf of @nicolehaugen by GitHub Copilot (model: GPT-5.4, autonomous).

[nicolehaugen]

@copilot Two small changes in this PR's helper cluster around `_iter_core_candidates`. Keep it tight — do not modify pre-existing Spec Kit code.

1. Reuse `PresetResolver.templates_dir` in `_project_core_asset_root`.

   File: `src/specify_cli/artifacts/__init__.py` (around lines 144-155).
   The current body duplicates `project_root / ".specify" / "templates"`, which is already exposed as `PresetResolver.templates_dir` (see `src/specify_cli/presets/__init__.py:5078`).

   Replace with:

   ```python
   def _project_core_asset_root(project_root: Path | None, subdir: str) -> Path | None:
   """Return the project-local built-in-tier directory for an asset family, if present."""
   if project_root is None:
   return None
   if subdir not in {"commands", "scripts", "templates"}:
   return None # pragma: no cover — internal misuse
   from ..presets import PresetResolver # lazy: avoids circular import
   candidate = PresetResolver(project_root).templates_dir
   if subdir != "templates":
   candidate = candidate / subdir
   return candidate if candidate.is_dir() else None

2. Rename PresetResolver.core_name_candidates to name_candidates.

File: src/specify_cli/presets/__init__.py (around lines 5294-5301). The behavior is tier-agnostic — it applies the Spec Kit-wide speckit.<stem> filename convention (see pre-existing _core_stem uses at :194, :2334, :2590, :5411-5419, :5765-5774). The tier label in its name is inaccurate.

Rename the classmethod and rewrite the docstring:

Python

@classmethod
def name_candidates(cls, logical_name: str) -> list[str]:
    """Return exact-first filename candidates for a `speckit.<stem>` logical name.

    Given `speckit.plan`, returns `["speckit.plan", "plan"]` so callers can try
    the fully-qualified filename first and fall back to the bare stem. Names
    that do not follow the `speckit.<stem>` convention return a single-element
    list containing the original name.
    """
    ...  # body unchanged

Update the single caller in src/specify_cli/artifacts/__init__.py (around line 859) from PresetResolver.core_name_candidates(name) to PresetResolver.name_candidates(name). Update any test/comment references. Do not add a shim/alias.

2. Add a shim comment above _iter_core_candidates and its two remaining tier-scoped helpers.

The functions _iter_core_candidates, _core_command_logical_name, and _project_core_asset_root are compatibility shims that exist only because the built-in tier has no manifest/registry today. Once the built-in tier is repackaged as a preset/extension with a manifest, _iter_candidate_artifacts will enumerate it like any other tier, and this whole cluster deletes cleanly. Their tier-scoped names are intentional.

Above _iter_core_candidates (around line 838), add:

Python

# Compatibility shim: the built-in tier has no manifest/registry, so we walk
# its filesystem convention here. Once the built-in tier is repackaged as a
# preset/extension, the registry-driven `_iter_candidate_artifacts` above
# will enumerate it uniformly and this helper (plus `_project_core_asset_root`
# and `_core_command_logical_name`) can be deleted.

No comment needed on the other two — one anchor at _iter_core_candidates is enough since it references them by name.

3. Run:

[Copilot]

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

src/specify_cli/_identifier.py:106

• The documented project sentinel is not enforced: derive_named_id("project", "foo", ...) emits project:foo:..., and layer_kind_from_lookup_id() accepts it even though project IDs require sourceId == "_". Enforce the sentinel in both construction and full-shape parsing so malformed project IDs cannot be treated as authoritative.

    if layer not in _LAYER_KINDS:
        raise IdentifierComponentError(f"Invalid layer '{layer}'")
    if kind not in _NAMED_CONTRIBUTION_KINDS:
        raise IdentifierComponentError(f"Invalid named contribution kind '{kind}'")
    validate_component(name, "name")

src/specify_cli/_identifier.py:47

• This parenthetical contradicts the intentional built-in contract stated above: built-in layers omit lookupId, so not every layer carries one. Narrow the statement to non-built-in layers.

its ``lookupId`` uses this label so the round-trip invariant (every layer
carries a ``lookupId``) still holds. No manifest ``iter_contributions()`` will

src/specify_cli/presets/init.py:5743

• The PR description still says collect_all_layers() uses the on-disk extension directory as sourceId, but this branch deliberately replaces it with the validated manifest ID for declared contributions. Update the PR description to match the implemented round-trip contract; the current text tells consumers the opposite identity to expect.

                            ext_manifest = ExtensionManifest(ext_manifest_path)
                            if isinstance(ext_manifest.id, str) and ext_manifest.id:
                                source_id_for_lookup = ext_manifest.id

src/specify_cli/_identifier.py:55

• The PR description still promises contribution IDs for every core contribution and a lookupId on each resolver layer, while this allow-list intentionally excludes core and the resolver omits built-in lookup IDs. Update the PR body to describe the source-agnostic built-in contract already implemented and documented here.

_LAYER_KINDS = frozenset({PROJECT_OVERRIDE_LAYER, "preset", "extension"})
_CONTRIBUTION_KINDS = frozenset({"command", "template", "script", "hook"})
_NAMED_CONTRIBUTION_KINDS = _CONTRIBUTION_KINDS - {"hook"}
_HOOK_LAYERS = frozenset({"preset", "extension"})

• Files reviewed: 22/22 changed files
• Comments generated: 1
• Review effort level: Balanced

[nicolehaugen]

@copilot Fix a stale-registry crash in _iter_convention_contributions in src/specify_cli/artifacts/__init__.py by modeling it after the pre-existing "skip missing pack dir" pattern already established elsewhere in Spec Kit.

Problem. If a preset/extension registry entry points to a directory that has been deleted from disk, the unguarded sorted(pack_dir.iterdir(), ...) call at the end of _iter_convention_contributions raises FileNotFoundError and takes down the entire specify artifact list / specify artifact info command. The subdirectory probes above it are already guarded with if not candidate_dir.is_dir(): continue, but the root-level walk is not.

Pre-existing precedent to model after. Spec Kit's own preset/extension resolution already handles this case gracefully — a stale registry entry is silently skipped, never fatal. See:

• PresetResolver._get_manifest(pack_dir) returns None for missing dirs; callers do if manifest is None: continue (used throughout src/specify_cli/presets/__init__.py in resolve paths and layer collectors).
• PresetManager.list_installed(): uses if not manifest_path.exists(): continue.
• PresetResolver.collect_all_layers(): silently drops missing packs, which is why preset list / preset resolve keep working over stale entries.

_iter_convention_contributions should follow the same rule.

Fix. Guard the pack root before the root-level walk, matching the style of the subdir guards immediately above:

if not pack_dir.is_dir():
    return
for entry in sorted(pack_dir.iterdir(), key=lambda p: p.name):
    ...

Place the guard right before the root-level for entry in sorted(pack_dir.iterdir(), ...) loop. Keep the existing subdirectory-loop guards as-is. Do not add error logging — the pre-existing pattern is silent skip.

Test. Add a test in tests/test_artifacts.py that:

1. Registers a preset (or extension) in the registry with an on-disk pack_dir.

2. Deletes the directory from disk without touching the registry entry.

3. Asserts specify artifact list and specify artifact info <a-real-artifact> succeed and simply omit the stale pack — do not raise FileNotFoundError and do not fail the whole inventory.

4. This mirrors the behavior of preset list in the same scenario.

Run:

Plain text

.venv/bin/python -m pytest tests/test_artifacts.py -q

[Copilot]

Addressed in e7bdc62: guarded the root-level pack_dir.iterdir() walk in _iter_convention_contributions with if not pack_dir.is_dir(): return, matching the subdir guards above. Added test_stale_registry_entry_with_missing_pack_dir_is_skipped covering a registered preset whose pack_dir is deleted after registration. 86 artifact tests pass, ruff clean.

[Copilot]

@copilot Two small changes in this PR's helper cluster around `_iter_core_candidates`. Keep it tight — do not modify pre-existing Spec Kit code.

1. Reuse `PresetResolver.templates_dir` in ...

Addressed in e67b1cd: _project_core_asset_root now reuses PresetResolver(project_root).templates_dir instead of duplicating the .specify/templates path. The core_name_candidates → name_candidates rename was already applied in bd483da.

[Copilot]

Review details

Suppressed comments (7)

Previously missed (2) — in code that hasn't changed since the last review.

src/specify_cli/_identifier.py:49

• “Every layer carries a lookupId” contradicts the intentional built-in contract in this module and the artifact docs: built-in layers omit lookupId. Scope the statement to project overrides so the identifier documentation remains internally consistent.

This issue also appears in the following locations of the same file:

• line 99
• line 145

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.

src/specify_cli/artifacts/init.py:488

• An unreadable extension registry can escape this public catalog boundary as raw OSError, because ExtensionRegistry(...) reads it before is_corrupt() is reached. Wrap construction just as the preset validator must, so all registry read failures consistently become ArtifactResolutionError.

This issue also appears on line 505 of the same file.

    from ..extensions import ExtensionRegistry

    if ExtensionRegistry(extensions_dir).is_corrupt():
        raise ArtifactResolutionError()

src/specify_cli/_identifier.py:147

• The parser's “complete shape” validation still accepts impossible source/layer pairs such as project:foo:template:x and preset:_:template:x. Mirror the constructor's source-ID invariant here so malformed external lookup IDs return None.

    layer = parts[0]
    if layer not in _LAYER_KINDS:
        return None

src/specify_cli/_identifier.py:106

• The constructor does not enforce the documented source-ID rule: it accepts project:anything:... and also _ for preset/extension layers. Those IDs do not represent valid sources, yet layer_kind_from_lookup_id() can treat them as authoritative. Enforce _ iff the layer is project.

    validate_component(layer, "layer")
    validate_component(source_id, "sourceId")
    validate_component(kind, "kind")
    if layer not in _LAYER_KINDS:
        raise IdentifierComponentError(f"Invalid layer '{layer}'")
    if kind not in _NAMED_CONTRIBUTION_KINDS:
        raise IdentifierComponentError(f"Invalid named contribution kind '{kind}'")
    validate_component(name, "name")

src/specify_cli/artifacts/init.py:508

• This validator can leak OSError instead of the catalog's ArtifactResolutionError: PresetRegistry(...) calls _load() before is_corrupt(), and a directory at .registry deterministically raises IsADirectoryError there. Wrap construction so direct ArtifactCatalog callers receive the same logical error as malformed registries and the CLI envelope.

    from ..presets import PresetRegistry

    if PresetRegistry(presets_dir).is_corrupt():
        raise ArtifactResolutionError()

docs/reference/artifacts.md:131

• This cause is stale: artifact content is no longer composed by list or info, so composition failure does not produce this error. Document the implemented cases—corrupt registries, filesystem failures, and resolver layer-collection errors.

| `artifact resolution failed`                        | The preset/extension registries could not be read, or artifact content could not be composed |

src/specify_cli/presets/init.py:5743

• The current PR description says extension sourceId uses the on-disk directory name, but this branch intentionally uses ExtensionManifest.id for declared entries and only falls back to the directory for convention-only entries. Update the PR description because it currently publishes the opposite lookup-ID contract.

                            ext_manifest = ExtensionManifest(ext_manifest_path)
                            if isinstance(ext_manifest.id, str) and ext_manifest.id:
                                source_id_for_lookup = ext_manifest.id

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (5)

Previously missed (2) — in code that hasn't changed since the last review.

src/specify_cli/_identifier.py:105

• The constructor does not enforce the documented project-layer source invariant: derive_named_id("project", "anything", "command", "x") returns project:anything:command:x, and layer_kind_from_lookup_id() accepts it even though project IDs require sourceId == "_". Enforce this relation in both construction and parsing so every accepted ID conforms to the declared grammar.

    if layer not in _LAYER_KINDS:
        raise IdentifierComponentError(f"Invalid layer '{layer}'")
    if kind not in _NAMED_CONTRIBUTION_KINDS:
        raise IdentifierComponentError(f"Invalid named contribution kind '{kind}'")

tests/test_contribution_ids.py:6

• This module summary says every resolved layer has a lookupId, but the new built-in-layer tests explicitly require core rows to omit it. Describe only project/preset/extension provenance layers as carrying lookup IDs so the test documentation matches the contract it asserts.

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

docs/reference/artifacts.md:131

• This cause is not implemented: artifact inspection never composes content, and layer-backed artifacts remain visible even when content cannot be decoded or composed. ArtifactResolutionError is used for corrupt registries, filesystem failures, or resolver errors while collecting layers, so document those actual causes instead.

| `artifact resolution failed`                        | The preset/extension registries could not be read, or artifact content could not be composed |

src/specify_cli/presets/init.py:5743

• The current PR description says extension sourceId uses the on-disk directory name, but this code deliberately replaces it with ExtensionManifest.id; the renamed-directory tests and reference docs assert that manifest identity is the contract. Update the PR description so it does not publish the opposite lookup-ID behavior.

                            ext_manifest = ExtensionManifest(ext_manifest_path)
                            if isinstance(ext_manifest.id, str) and ext_manifest.id:
                                source_id_for_lookup = ext_manifest.id

src/specify_cli/_identifier.py:47

• The parenthetical contradicts this PR's intentional built-in contract: built-in stack layers omit lookupId, so it is not true that every layer carries one. Limit the invariant to provenance-backed/project-override layers.

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

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[nicolehaugen]

@copilot Change `specify artifact list --json` to always include the `stack` array per artifact — matching the shape `bundle list --json` uses (full record per row, no summary/detail split, no opt-in flag).

Why

**Precedent — `bundle list --json` returns full records.** See `bundler/models/records.py:44-52` (`InstalledBundleRecord.to_dict()`) — one call, full record shape including `contributed_components`. There is no `bundle info` fallback needed for consumers that want provenance.

**Real usage — wizard boot regression.** The Spec Kit Wizard canvas boot currently calls `list --json` then loops `info --json` per artifact — 68 shell-outs in a project with a few presets + an extension. Each shell-out pays Python cold-start (~400–900ms on Windows). Total boot cost: 27–60s. This regression is a direct consequence of `list`'s summary shape, and it disappears the moment `stack` is inlined.

**No back-compat risk.** The `artifact` command is being introduced by this same PR. There are no pre-existing external callers to break. `list --json`'s response shape is being expanded, not renamed or narrowed.

Change

**File:** `src/specify_cli/artifacts/_commands.py`

`artifact_list` (currently around lines 95-118): when `--json` is passed, each row must include `stack` — the same array shape `get_artifact_info` returns today, unchanged. Result per row:

```json
{
"id": "command:speckit.plan",
"name": "speckit.plan",
"kind": "command",
"description": "Execute the implementation planning workflow...",
"stack": [
{
"id": "command:speckit.plan",
"layer": null,
"sourceId": null,
"presetId": null,
"presetName": null,
"strategy": "replace",
"active": true,
"hidden": false,
"manifestPath": null,
"lookupId": null
}
]
}

That's exactly what info returns today — so list --json now returns [<info-shape>, <info-shape>, ...].

Implementation — reuse, don't re-do

Do not re-resolve layers per artifact after listing. ArtifactCatalog.list_artifacts() at artifacts/__init__.py:~640 already computes the full layers_cache: dict[(kind, name), list[layer]] in one pass while iterating and returns (artifacts, layers_cache). That cache is the exact data needed to emit stack. Cost: zero extra work — just serialize what's already in memory.

Share the layer-serialization function between list and info. If the layer-shape helper is currently inlined inside get_artifact_info, factor it out into a top-level module function (e.g. _serialize_layer(layer) or _public_layer_dict(layer) — whatever fits the existing naming style) and call it from both list and info. This is important: the two commands must produce identical stack shapes because we're guaranteeing at the API level that list's per-row output deep-equals info's output for the same id. Copy-paste guarantees drift eventually; a shared helper prevents it.

Keep info — do not deprecate it

info stays for single-artifact ergonomics — humans debugging one artifact, or a programmatic caller who only wants one row. Its output becomes literally list --json-row-for-that-id, but the CLI ergonomics matter (no need to shell out list --json | jq '.[] | select(.id=="...")').

Add a one-liner to info's docstring noting it returns the same shape as one row of list --json for callers wondering whether to prefer one over the other.

Do NOT touch

• list's no---json (text) mode — that intentionally errors out per _require_json_flag today. Same behavior after this change. Rendering stacks in text mode is a separate design question.
• info shape — must stay a superset-of-nothing of a list row. Same shape, same fields, same order.
• The internal ArtifactCatalog.resolve_content() method — not exposed as a CLI subcommand and out of scope.
• Any preset/extension code paths.

Tests

Add to the existing artifact command tests (find the file — probably tests/artifacts/test_commands.py or wherever artifact_list / artifact_info are already exercised; don't create a new file):

1. list --json emits stack on every row. Each row has all the same keys as info output — same list of keys, same nesting.
2. Deep-equal parity: list --json row for id X == info --json output for id X. This is the anti-drift guard for the shared serializer. Do this for at least one built-in and one preset-provided artifact (fixture a minimal preset if none exists in the test setup).
3. Cache correctness: no duplicate layer resolution. Not a functional test — a structural one that guards the "reuse layers_cache" invariant. Simplest form: call list --json and verify ArtifactCatalog.list_artifacts() is invoked once, not once + N collect_all_layers calls. If a spy/mock harness is already in the test infrastructure, use it; otherwise leave a comment marking the invariant and skip the test rather than build new mock infrastructure.
4. info --json shape unchanged (regression guard — its output should already match a list row, but pin it).

Governing principles reminder

Follow the "reuse existing patterns" rule from this PR's review sweep:

• Match bundle list --json's full-record-per-row shape.
• Share the layer-serialization helper between list and info — one function, both call it.
• Reuse the existing layers_cache from list_artifacts() — don't re-resolve per artifact.
• No new abstractions unless they simplify existing code.

Confirm with me before diverging from any of these — e.g., if the layer helper is inlined in get_artifact_info in a way that makes extraction non-trivial, or if there's an unrelated reason list and info produce different layer shapes today, flag it and ask.

[Copilot]

@copilot Change `specify artifact list --json` to always include the `stack` array per artifact — matching the shape `bundle list --json` uses (full record per row, no summary/detail split, no...

Implemented in 9977faa.

artifact list --json now returns full records with stack included on every row, using the same stack row shape as artifact info. I also added a CLI test to assert the stack field is present and matches artifact info for the same artifact.

Posted on behalf of @nicolehaugen by GitHub Copilot (model: GPT-5 Codex).

[Copilot]

Review details

Suppressed comments (4)

Previously missed (3) — in code that hasn't changed since the last review.

src/specify_cli/_identifier.py:105

• The documented grammar requires sourceId == "_" for project layers, but this constructor only checks that it is non-empty and colon-free. As a result, derive_named_id("project", "other", "template", "x") succeeds and layer_kind_from_lookup_id() also accepts the invalid ID. Enforce the project/source relationship in both construction and parsing so the full-shape validation contract remains symmetric.

    if layer not in _LAYER_KINDS:
        raise IdentifierComponentError(f"Invalid layer '{layer}'")
    if kind not in _NAMED_CONTRIBUTION_KINDS:
        raise IdentifierComponentError(f"Invalid named contribution kind '{kind}'")

docs/reference/artifacts.md:131

• This cause is not implemented: the artifact catalog no longer composes content when serving list or info, so composition failure does not produce this error. Document the actual paths mapped to ArtifactResolutionError: corrupt registries, filesystem access failures, or resolver errors while collecting layers.

| `artifact resolution failed`                        | The preset/extension registries could not be read, or artifact content could not be composed |

src/specify_cli/_identifier.py:49

• The “every layer carries a lookupId” invariant is false under this module's intentional built-in contract: lines 25-26 and both resolver core branches omit lookupId for built-in layers. Reword this to describe the synthetic project-override identity without claiming it applies to every stack 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.

src/specify_cli/presets/init.py:5729

• The current PR description says the on-disk extension directory is used as sourceId for manifest-declared entries, but this implementation deliberately uses the validated manifest ID and reserves directory identity for convention-only contributions. Update the PR description so it does not publish the opposite opaque-ID contract.

                # Manifest-declared entries use the manifest's validated ``id:``
                # for the lookupId's sourceId, so ``lookupId`` joins directly to
                # ``ExtensionManifest.iter_contributions()``'s ``id`` even when
                # the installed directory (``ext_id``) was renamed. Convention-
                # only contributions have no manifest to consult and fall back
                # to the directory identity. The directory identity is retained

• Files reviewed: 22/22 changed files
• Comments generated: 1
• Review effort level: Balanced

[nicolehaugen]

@copilot update contract to reflect new shape

[Copilot]

Updated in d9d1ef7: the artifact reference now documents artifact list --json as returning full records with stack per row, and aligns the field table / artifact info cross-reference with that shape. Verified with .venv/bin/python -m pytest tests/test_artifact_command.py -q and uvx ruff@0.15.0 check docs/reference/artifacts.md.

Posted on behalf of @nicolehaugen by GitHub Copilot (model: GPT-5.6, autonomous).

[Copilot]

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

src/specify_cli/_identifier.py:105

• The documented grammar requires project IDs to use the _ source sentinel, but this constructor accepts values such as project:other:template:x, and layer_kind_from_lookup_id() also classifies them as valid. Enforce source_id == "_" for the project layer in both construction and parsing so the shared API cannot create or accept IDs outside its own grammar.

    validate_component(layer, "layer")
    validate_component(source_id, "sourceId")
    validate_component(kind, "kind")
    if layer not in _LAYER_KINDS:
        raise IdentifierComponentError(f"Invalid layer '{layer}'")
    if kind not in _NAMED_CONTRIBUTION_KINDS:
        raise IdentifierComponentError(f"Invalid named contribution kind '{kind}'")

src/specify_cli/artifacts/init.py:303

• This returns a manifestPath whenever the pack happens to contain a manifest, even for a convention-only artifact that the manifest does not declare. That makes the JSON contradict its documented “declaring manifest” meaning and points consumers to a file with no matching contribution. Preserve whether the resolver match was manifest-declared (or verify the lookup ID against iter_contributions) and return null for undeclared convention rows.

    if isinstance(pack_dir, Path):
        manifest_path = pack_dir / manifest_name
    elif pack_id:
        manifest_path = project_root / ".specify" / tier_dir / pack_id / manifest_name
    else:
        return None
    if not manifest_path.is_file():

src/specify_cli/presets/init.py:5743

• The current PR description still says extension sourceId comes from the on-disk directory, but this branch deliberately emits the validated manifest ID for declared entries (and the renamed-extension tests assert that contract). Please update the description to reserve directory identity for convention-only contributions; otherwise it publishes the opposite lookup-ID behavior.

                            ext_manifest = ExtensionManifest(ext_manifest_path)
                            if isinstance(ext_manifest.id, str) and ext_manifest.id:
                                source_id_for_lookup = ext_manifest.id

src/specify_cli/artifacts/init.py:675

• artifact list calls collect_all_layers() once per candidate, while each call walks every extension and reparses each extension.yml (and reparses the matching manifest again for its source ID). With A artifacts and E extensions this produces O(A×E) YAML parses. Cache extension manifests on the resolver, as presets already are, or build the stack index in one pass before listing.

            if key not in layers_cache:
                try:
                    layers_cache[key] = resolver.collect_all_layers(name, kind)
                except (OSError, PresetError) as exc:

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Balanced