feat: expose hook contributions via specify artifact by nicolehaugen · Pull Request #4348 · github/spec-kit

nicolehaugen · 2026-08-27T03:54:09Z

Summary

expose hook contributions through specify artifact list --json and specify artifact info --json
group declarations by hook:{eventName}:{targetCommand} and preserve deterministic per-contributor lookupId values
model duplicate declarations as additive execution, matching the existing HookExecutor runtime
project runtime registration state into registered and per-entry active without hiding declared-but-unregistered hooks
preserve the common artifact stack provenance and existing manifest/registry error contracts
document and test hook shorthand, activation, ordering, provenance, error behavior, and existing-kind compatibility

Hook JSON contract

Hook artifacts contain one stack entry per declaring contributor. Each entry uses strategy: "additive"; multiple entries may be active: true because Spec Kit executes every enabled hook returned by HookExecutor.get_hooks_for_event(). Per-entry priority and optional come from the declaring manifest. Top-level registered is true when any stack entry is active.

Runtime registration lookup deliberately follows Spec Kit's existing tolerant hook configuration behavior: invalid or unreadable .specify/extensions.yml content is normalized to an empty registration map, leaving declared hooks visible with registered: false. Manifest, extension-registry, and artifact-layer collection failures retain the existing artifact resolution failed envelope.

This replaces the issue's original winner/replacement proposal after implementation review confirmed that a single active winner would contradict existing Spec Kit hook behavior. The contract in #4343 has been revised accordingly.

Stack

Stacked on top of #4305.

Closes #4343

Authored on behalf of @nicolela by GitHub Copilot (model: GPT-5.6 Sol).

Extends `specify artifact list --json` and `specify artifact info --json` to surface hook contributions as a fourth ArtifactKind alongside command / template / script. Adds a parallel iterator + stack builder for hooks (their name legitimately contains `:`, breaking the existing (kind, name) tuple grammar), preserving strictly additive JSON envelope changes for the existing three kinds. Hook rows carry top-level `eventName`, `targetCommand`, `optional`, `priority`, and `registered` fields. Stack entries use `strategy: replace` and `lookupId` from `derive_hook_id`. The `registered` flag mirrors the runtime by reading `.specify/extensions.yml` bindings via a new `HookExecutor.is_hook_registered` helper. Declared-but-unbound hooks still appear with `registered: false`. The shorthand `hook:{eventName}:{targetCommand}` round-trips through `artifact info`. Includes 30 new tests covering surfacing, sort order, active-winner selection, registered semantics, the layer invariant, and no-regression on existing kinds. Docs at `docs/reference/artifacts.md` extended with a Hook artifacts subsection. Closes #4343 Assisted-by: GitHub Copilot (model: Claude Opus 4.7, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

Copilot

Pull request overview

Adds hook contributions to the artifact introspection API alongside commands, templates, and scripts.

Changes:

Adds hook inventory, stack, lookup, ordering, and registration metadata.
Adds runtime registration helper and extensive tests.
Documents hook artifact fields and semantics.

Show a summary per file

File	Description
`src/specify_cli/artifacts/__init__.py`	Implements hook artifact collection and serialization.
`src/specify_cli/artifacts/_commands.py`	Accepts hooks in CLI help and kind validation.
`src/specify_cli/extensions/__init__.py`	Adds hook registration lookup.
`tests/test_artifact_command.py`	Covers hook inventory and CLI behavior.
`docs/reference/artifacts.md`	Documents hook artifacts.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (1)

docs/reference/artifacts.md:183

Correct the grammatical error in this field description.

| `eventName`     | The event whose fires trigger this hook (`before_specify`, `after_plan`, …)                     |

Files reviewed: 5/5 changed files
Comments generated: 7
Review effort level: Balanced

mnriem

Please address Copilot feedback and fix test & lint errrors

The hook stack builder consumes the manifest-emitted `id` from `EnhancedManifest.iter_contributions()` directly, so the top-level `derive_hook_id` import is unused. Assisted-by: GitHub Copilot (model: Claude Opus 4.7, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

Copilot

Copilot review overview

🔵 Needs a closer look

Hook activity and registration metadata can disagree with runtime behavior, and required stack provenance is missing.

Review tier: Balanced
Findings: 5 · 1

Pre-existing issues (6)

Severity	Finding
	`src/specify_cli/artifacts/__init__.py` — `is_hook_registered()` reloads and parses `.specify/extensions.yml` on every call, and this loop… View comment
	`src/specify_cli/artifacts/__init__.py` — The issue's proposed hook JSON keeps the common stack fields (`presetId`, `presetName`, `hidden`,… View comment
	`src/specify_cli/artifacts/__init__.py` — Hook collection does not preserve the artifact command's error contract. An `OSError` while… View comment
	`src/specify_cli/extensions/__init__.py` — This differs from the runtime filter in `get_hooks_for_event`, which treats any falsy `enabled`… View comment
	`src/specify_cli/artifacts/__init__.py` — Duplicate declarations are not winners at runtime: `HookExecutor.register_hooks` preserves entries… View comment
	`docs/reference/artifacts.md` — This says every kind is sorted by name, but the new hook implementation sorts hooks by event and… View comment

Issues resolved since last review (1)

Severity	Finding
	`src/specify_cli/artifacts/__init__.py` — This import is never referenced in the module. The repository runs Ruff over `src`, so this… View resolved comment

Suppressed comments (7)

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

docs/reference/artifacts.md:183

Correct the grammar in this field description.
tests/test_artifact_command.py:1349
This contract excludes the wildcard binding shape used below by test_binding_without_command_matches. State that command is optional so the helper documentation matches the supported input.

src/specify_cli/extensions/init.py:5280

This check disagrees with the runtime filter in get_hooks_for_event(), which excludes every falsey enabled value. For example, enabled: null is not executed at runtime but is reported as registered: true here. Use the same truthiness rule so registered reflects execution state.

            if entry.get("enabled", True) is False:

src/specify_cli/artifacts/init.py:536

The runtime does not select a single winner for duplicate (event, command) declarations: register_hooks() retains one binding per extension, and get_hooks_for_event() returns all enabled bindings in priority order. Marking only the first contributor active (and projecting only its optional/priority) therefore reports later hooks as inactive even though check_hooks_for_event() will expose them for execution. Either add matching runtime deduplication or model every executable contributor without a single-winner claim.

                layer=layer,  # type: ignore[arg-type]
                sourceId=str(source_id),
                strategy="replace",
                active=(position == 0),
                lookupId=str(lookup_id),

src/specify_cli/artifacts/init.py:1086

is_hook_registered() calls get_project_config() internally, so this comprehension rereads and reparses the entire YAML file once per stack entry. Inventory generation becomes an N+1 I/O path and can approach quadratic work as hooks/bindings grow. Load the normalized config once and match all entries against that snapshot (or let the helper accept it).

            registered = any(
                hook_executor.is_hook_registered(
                    event_name=event_name,
                    extension_id=entry.sourceId,
                    command=command,
                )
                for entry in stack_entries

src/specify_cli/artifacts/init.py:165

Issue #4343's proposed hook stack retains the common presetId, presetName, hidden, and manifestPath fields, but this new type drops all four. In particular, manifestPath is meaningful provenance for a manifest-declared hook, and omitting the preset fields makes the reserved preset layer unable to identify its installed pack. Preserve the established stack shape and use null only where a field is genuinely inapplicable.

    id: str
    layer: LayerName
    sourceId: str
    strategy: Literal["replace"]
    active: bool
    lookupId: str
    priority: int
    optional: bool

docs/reference/artifacts.md:19

This says all kinds are sorted by name, but hook rows are actually sorted by event and winner priority, as the new Sort order section later explains. Document the separate hook ordering here to avoid a contradictory contract.

Prints the full inventory of every visible artifact — one row per `(kind, name)` pair, including its composition `stack` — sorted by kind (`command`, then `template`, then `script`, then `hook`) and then by name.

Reuse HookExecutor.get_hooks_for_event() so hook artifact stacks reflect the runtime's enabled bindings instead of synthesizing a single priority winner. Duplicate declarations from different extensions remain active and execute additively in priority order. Remove row-level priority and optional fields because no single contributor owns those values. Keep them per stack entry, mark each contributor active independently, and report top-level registered when any contributor is active. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

Define hook stack entries as active exactly when their declaration matches an enabled registration returned by HookExecutor.get_hooks_for_event(). Document that priority winners and event-time condition evaluation do not affect this flag. Add one focused command-matching regression test and extend existing registration tests with active-state assertions without duplicating their setup. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

Document that hook priority and optionality come from the contributing manifest while active state comes from HookExecutor.get_hooks_for_event(). Clarify that this split follows Spec Kit's existing hook registration behavior and does not attempt to solve later configuration drift. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

Let hook resolver failures reach the collection boundary and translate OSError and PresetError to the existing ArtifactResolutionError, matching the named-artifact inventory path. This prevents resolver failures from appearing as unknown hooks or escaping the catalog contract. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

Keep presetId, presetName, hidden, and manifestPath on hook stack entries so hooks follow the existing artifact stack contract. Reuse the shared manifest path and preset display-name helpers, with hidden fixed false for additive hooks. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

Clarify the separate hook ordering rule, correct the eventName field description, and document that test hook bindings may omit command for the supported wildcard shape. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

Copilot

Copilot review overview

🟡 Changes recommended

The implementation diverges from the accepted JSON contract and contains registration matching and ordering defects.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 · 1 · 1

New issues introduced by this change (2)

Severity	Finding
	`src/specify_cli/artifacts/__init__.py` — The hook JSON model here contradicts both this PR description and issue #4343's accepted contract.…
	`src/specify_cli/artifacts/__init__.py` — This insertion index comes from resolver order, not runtime registration order.…

Pre-existing issues (1)

Severity	Finding
	`docs/reference/artifacts.md` — This says every kind is sorted by name, but the new hook implementation sorts hooks by event and… View comment

Issues resolved since last review (5)

Severity	Finding
	`src/specify_cli/artifacts/__init__.py` — `is_hook_registered()` reloads and parses `.specify/extensions.yml` on every call, and this loop… View resolved comment
	`src/specify_cli/artifacts/__init__.py` — The issue's proposed hook JSON keeps the common stack fields (`presetId`, `presetName`, `hidden`,… View resolved comment
	`src/specify_cli/artifacts/__init__.py` — Hook collection does not preserve the artifact command's error contract. An `OSError` while… View resolved comment
	`src/specify_cli/extensions/__init__.py` — This differs from the runtime filter in `get_hooks_for_event`, which treats any falsy `enabled`… View resolved comment
	`src/specify_cli/artifacts/__init__.py` — Duplicate declarations are not winners at runtime: `HookExecutor.register_hooks` preserves entries… View resolved comment

Suppressed comments (5)

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

src/specify_cli/artifacts/init.py:778

A valid bare hook whose event is named command, template, or script cannot resolve: this branch interprets command:foo as named-artifact shorthand even when kind="hook", then rejects the mismatch. Hook event names use the general identifier-component validator and do not reserve these words, while the new API documents every {eventName}:{targetCommand} as an accepted bare name. When the explicit kind is hook, preserve the whole input as the hook name unless it starts with hook:.
docs/reference/artifacts.md:199
Correct the grammatical error in this field description.

src/specify_cli/artifacts/init.py:445

This preset branch is currently unreachable: PresetManifest.iter_contributions() only emits command, template, and script from provides.templates (src/specify_cli/presets/__init__.py:547-570). Therefore the PR does not actually surface preset-layer hooks or test them, despite claiming forward-compatible preset surfacing and issue #4343 requiring hooks from installed presets. Extend the preset schema/contribution API to emit hooks, or narrow the stated contract instead of retaining a permanently empty loop.

    # Presets are walked first for forward compatibility with a future
    # ``PresetManifest.iter_contributions()`` that emits hooks. Today none
    # do, so this loop yields nothing — but the ordering ensures that if a
    # preset ever declares a hook it participates in the same insertion-index
    # tiebreak as extensions.

src/specify_cli/artifacts/init.py:559

The wildcard test uses truthiness, so malformed bindings such as command: "", command: null, or command: 0 mark every declared command for this extension/event active. The stated rule only treats an omitted command key as a wildcard; distinguish absence from a present invalid value so malformed configuration degrades to unregistered rather than a false positive.

        active = any(
            binding.get("extension") == source_id
            and (
                not binding.get("command")
                or binding.get("command") == command
            )
            for binding in enabled_bindings

docs/reference/artifacts.md:19

This says every kind is sorted by name, but hook rows are actually sorted by event and first-entry priority, as the new Sort order section later states. Clarify the split ordering so consumers do not rely on lexicographic hook-name order.

Prints the full inventory of every visible artifact — one row per `(kind, name)` pair, including its composition `stack` — sorted by kind (`command`, then `template`, then `script`, then `hook`) and then by name.

+    Hook stack entries preserve the common :class:`StackLayer` fields and add
+    ``priority`` and ``optional`` — the two per-contributor scalars that vary
+    across hook declarations. ``strategy`` is fixed to
+    ``"additive"`` because enabled hooks from different extensions all run;
+    priority orders them but does not select a winner. ``hidden`` is therefore


+    def _sort_key(item: tuple[int, dict[str, Any]]) -> tuple[int, int]:
+        idx, contribution = item
+        priority = normalize_priority(
+            contribution.get("priority"), DEFAULT_HOOK_PRIORITY
+        )
+        return (priority, idx)
+
+    ordered = sorted(grouped, key=_sort_key)


Merge the refreshed contribution-artifacts base and remove the obsolete preset registry validator call from the hook inventory path. Mark manifest-derived hook contributions explicitly so the shared manifest-path helper preserves their provenance. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

Copilot

Copilot review overview

🟡 Changes recommended

Disabled hook declarations are omitted, and runtime-config I/O failures are silently treated as unregistered hooks.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Balanced
Findings: 1 · 3

New issues introduced by this change (2)

Severity	Finding
	`src/specify_cli/artifacts/__init__.py` — `iter_extensions_by_priority()` explicitly filters disabled registry entries…
	`src/specify_cli/artifacts/__init__.py` — This catch cannot enforce the documented filesystem-error contract for `.specify/extensions.yml`:…

Pre-existing issues (2)

Severity	Finding
	`src/specify_cli/artifacts/__init__.py` — The hook JSON model here contradicts both this PR description and issue #4343's accepted contract.… View comment
	`src/specify_cli/artifacts/__init__.py` — This insertion index comes from resolver order, not runtime registration order.… View comment

Issues resolved since last review (1)

Severity	Finding
	`docs/reference/artifacts.md` — This says every kind is sorted by name, but the new hook implementation sorts hooks by event and… View resolved comment

+    for _priority, ext_id, metadata in resolver.iter_extensions_by_priority():
+        ext_dir = resolver.extensions_dir / ext_id
+        if metadata is not None:
+            manifest = ext_manager.get_extension(ext_id)


+                try:
+                    enabled_hooks_by_event[event_name] = (
+                        hook_executor.get_hooks_for_event(event_name)
+                    )
+                except (OSError, PresetError) as exc:
+                    raise ArtifactResolutionError() from exc


Use the existing tolerant hook configuration loader for artifact registration state and document unreadable runtime config as unregistered rather than a resolution failure. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667230a1-e9fa-4500-a57f-c1c482be2507

nicolehaugen · 2026-09-09T22:20:36Z

Addressed the runtime-config error-contract feedback in commit \863f0dc4. Hook artifact registration now explicitly follows the existing tolerant \HookExecutor.get_project_config()\ behavior: invalid or unreadable .specify/extensions.yml\ is treated as an empty registration map, while manifest and extension-registry failures retain the artifact resolution error envelope. The ineffective caller-side exception translation was removed, the contract documentation and #4343 were aligned, and focused coverage now pins the unreadable-config behavior.\n\nPosted on behalf of @nicolela by GitHub Copilot (model: GPT-5.6 Sol).

Copilot

Copilot review overview

🔵 Needs a closer look

Command-less bindings are incorrectly treated as wildcards, and manifest failures do not preserve the required error contract.

Review tier: Balanced
Findings: 1 · 3

Pre-existing issues (4)

Severity	Finding
	`src/specify_cli/artifacts/__init__.py` — The hook JSON model here contradicts both this PR description and issue #4343's accepted contract.… View comment
	`src/specify_cli/artifacts/__init__.py` — This catch cannot enforce the documented filesystem-error contract for `.specify/extensions.yml`:… View comment
	`src/specify_cli/artifacts/__init__.py` — `iter_extensions_by_priority()` explicitly filters disabled registry entries… View comment
	`src/specify_cli/artifacts/__init__.py` — This insertion index comes from resolver order, not runtime registration order.… View comment

Suppressed comments (4)

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

src/specify_cli/artifacts/init.py:668

This path silently drops a registered extension whose manifest fails validation because get_extension() converts ValidationError to None; the fallback branch below also suppresses validation and read errors. As a result, malformed/unreadable hook manifests produce an omitted row (or unknown artifact) instead of the artifact resolution failed envelope required by the PR and #4343 error contract. Distinguish an absent manifest from a parse/read failure and let the latter be wrapped as ArtifactResolutionError.

This issue also appears on line 752 of the same file.
tests/test_artifact_command.py:1816

This test codifies an unsupported wildcard. Runtime registration always requires a command and skips entries without one; a manually malformed command-less entry is returned as a missing command, not expanded across the extension's declarations. Replace this with a regression asserting the declaration remains inactive.
docs/reference/artifacts.md:231
The documented command-omission wildcard does not exist in the hook runtime. register_hooks() skips entries without a command, and execution exposes such manually authored entries as a missing command rather than expanding them to every declared command. Document exact command matching so active describes a hook the runtime can actually invoke.

src/specify_cli/artifacts/init.py:756

A binding with no command is a malformed runtime hook, not an event-wide wildcard: register_hooks() skips such entries, while execution renders them as <missing command>/None. Treating a missing command as a match marks every declaration from this extension/event active even though none of those commands would be executed. Require exact command equality here; the added wildcard test and documentation should be updated accordingly.

            binding.get("extension") == source_id
            and (
                not binding.get("command")
                or binding.get("command") == command
            )

nicolehaugen requested a review from mnriem as a code owner August 27, 2026 03:54

Copilot AI balanced review requested due to automatic review settings August 27, 2026 03:54

Copilot started reviewing on behalf of nicolehaugen August 27, 2026 03:54 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

mnriem requested changes Sep 1, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 9, 2026 19:08

Copilot started reviewing on behalf of nicolehaugen September 9, 2026 19:09 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

nicolehaugen and others added 5 commits September 9, 2026 14:18

Copilot AI review requested due to automatic review settings September 9, 2026 21:14

Copilot started reviewing on behalf of nicolehaugen September 9, 2026 21:15 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 9, 2026 22:03

Copilot started reviewing on behalf of nicolehaugen September 9, 2026 22:03 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 9, 2026 22:20

Copilot started reviewing on behalf of nicolehaugen September 9, 2026 22:20 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat: expose hook contributions via specify artifact - #4348

feat: expose hook contributions via specify artifact#4348
nicolehaugen wants to merge 10 commits into
nicolehaugen-contribution-ids-and-artifactsfrom
nicolehaugen-expose-hook-artifacts

nicolehaugen commented Aug 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

nicolehaugen commented Sep 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

nicolehaugen commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Hook JSON contract

Stack

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mnriem left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

nicolehaugen commented Sep 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nicolehaugen commented Aug 27, 2026 •

edited

Loading