Add deterministic contribution IDs and stack lookup IDs for resolved artifacts

[nicolehaugen]

Fixes #4210

Summary

Every command, template, script, and hook contribution returned by preset and extension manifest surfaces now carries a computed opaque id, and every resolved artifact-stack layer carries a matching lookupId derived from the same recipe.

Identifier contract

• Named contributions: {layer}:{sourceId}:{kind}:{name} — layer ∈ core|preset|extension, sourceId = _ for core / preset id / extension id, kind ∈ command|template|script|hook, name = the declared name.
• Hook contributions: {layer}:{sourceId}:hook:{eventName}:{command}, with a :{12-hex} discriminator appended when two entries in the same source share the same (eventName, command) pair. Discriminator = first 12 lowercase hex chars of sha256(canonical_json(entry − {eventName, command})). Two hook entries whose remaining declared fields are byte-identical are rejected at manifest load — there is no meaningful way to distinguish them.
• : is reserved as the component separator and is now guarded on hook eventName/command. All other id-component fields are already regex-constrained to forbid it.
• Identifiers are computed at read/serialization/resolution time from author-declared manifest content only — no paths, timestamps, os.environ, or file-content hashes contribute — so they are stable across machines, reinstalls, and directory moves. Nothing is persisted.
• Project-local overrides carry a resolver-only sentinel project:_:{kind}:{name} lookupId that intentionally does not match any manifest contribution.

The change is purely additive: existing name-based resolution is preserved, and no production call-site keys off the new fields.

Modules touched

• src/specify_cli/_identifier.py — new pure derivation module (derive_named_id, derive_hook_id, hook_discriminator, canonical_json, validate_component, PROJECT_OVERRIDE_LAYER).
• src/specify_cli/extensions/__init__.py — hook : guards on eventName and command, byte-identical hook duplicate rejection, ExtensionManifest.iter_contributions() and ExtensionManifest.contribution_id().
• src/specify_cli/presets/__init__.py — PresetManifest.iter_contributions() and PresetManifest.contribution_id(), lookupId on every layer emitted by PresetResolver.collect_all_layers().

Tests added

• tests/test_contribution_ids.py — 39 tests covering the layer×kind derivation matrix, canonical JSON semantics, hook discriminator (no-collision, collision-with-suffix, reorder-stability, byte-identical rejection), : component guards, contribution-surface shape (additive, underlying data un-mutated), resolver lookupId round-trip for project/core/preset layers, cross-subprocess determinism, mtime independence, and non-persistence.

Docs updated

• extensions/EXTENSION-API-REFERENCE.md — new "Contribution Identifiers" section (grammar, hook convention, discriminator recipe, reserved character, project: sentinel, Python API, determinism guarantees, opacity guidance) and TOC entry.
• docs/reference/presets.md — new "Contribution Identifiers" section that cross-links to the extension reference for the full grammar.

Validation

pytest -q on the affected suites is clean apart from 10 pre-existing symlink-related failures caused by Windows privilege limitations (they fail identically on main without these changes). All 39 new tests pass; all extensions/presets/hooks/unit tests unaffected by symlinks pass.

Deviations from tasks.md

The scratch task list was drafted against an idealized layout that does not match the repo. Concrete adjustments applied in place:

• Identifier module lives at src/specify_cli/_identifier.py (top-level helper, matching the _download_security.py / _utils.py pattern) rather than an idealized src/specify_cli/manifests/_identifier.py. There is no shared manifests/ package here.
• Manifest validation guards and hook duplicate detection live inside the existing _validate() methods on ExtensionManifest (and, where relevant, PresetManifest) rather than in a separate manifests/validation.py / manifests/loader.py.
• Contribution surfaces are exposed as new iter_contributions() / contribution_id() methods returning dicts rather than as typed ContributionRef / HookContribution / ResolvedStackLayer dataclasses. Existing manifest storage stays untyped and byte-for-byte unchanged.
• Layer dicts returned by PresetResolver.collect_all_layers() gain a lookupId key. There is no ResolvedStackLayer type to add a field to.
• Test fixtures are built programmatically in the test module rather than checked in as an on-disk fixture tree; this matches the existing pattern in tests/test_extensions.py / tests/test_presets.py.
• The pre-feature pytest count sentinel is skipped — it is a CI heuristic, not a correctness check, and produces false positives on any unrelated test addition. Backward compatibility is asserted directly by leaving every existing test unmodified and by explicit additive-shape tests on the enriched contribution dicts.

[Copilot]

Pull request overview

Adds deterministic IDs for manifest contributions and lookup IDs for resolved artifact layers.

Changes:

• Adds identifier derivation and hook discrimination.
• Enriches preset/extension contribution and resolver APIs.
• Adds tests and identifier documentation.

Show a summary per file

File	Description
src/specify_cli/_identifier.py	Implements identifier derivation.
src/specify_cli/extensions/__init__.py	Adds extension contribution IDs and validation.
src/specify_cli/presets/__init__.py	Adds preset IDs and resolver lookup IDs.
tests/test_contribution_ids.py	Tests determinism and lookup behavior.
extensions/EXTENSION-API-REFERENCE.md	Documents the identifier contract.
docs/reference/presets.md	Documents preset contribution IDs.

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)

extensions/EXTENSION-API-REFERENCE.md:917

• The previous ## File System Layout heading was removed when this section was inserted, so the existing tree below is now an orphaned code block under “Opacity guidance.” Restore the heading before the tree to preserve the document structure.

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

[Copilot]

Review details

Suppressed comments (5)

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

extensions/EXTENSION-API-REFERENCE.md:918

• The previous File System Layout heading was replaced when this section was inserted, leaving the directory tree below as an orphaned code block. Restore the heading before the tree.

src/specify_cli/extensions/init.py:467

• Duplicate detection groups the raw hook command before the later canonicalization pass. An event containing both test-ext.hello and speckit.test-ext.hello with identical remaining fields passes this check, then both references are rewritten to the same command and iter_contributions() emits the same discriminator and ID for both. Group using the same canonical command form so normalization cannot create duplicate IDs.

                )
            if "name" not in cmd or "file" not in cmd:
                raise ValidationError("Command missing 'name' or 'file'")

src/specify_cli/presets/init.py:5628

• When entry is None, candidate can be a conventionally discovered preset file rather than a manifest contribution. This still emits a manifest-shaped preset:... ID, but PresetManifest.contribution_id() returns None for it, so the documented lookup round trip fails and project overrides are no longer the only non-matching layer. Represent and document undeclared preset fallbacks explicitly, as is attempted for extensions, or expose a corresponding synthetic contribution.

                        "source": f"{pack_id} v{version}",
                        "strategy": strategy,
                        "lookupId": derive_named_id(

src/specify_cli/extensions/init.py:879

• For colliding hooks, every entry has the same synthesized name (eventName:command), so this method always returns the first ID and provides no way to retrieve any discriminator-suffixed sibling. Make the hook lookup unambiguous—for example by accepting discriminator/declared fields or returning all matching IDs—instead of presenting this as a single-contribution lookup.

        ``name`` is the declared name for command/template/script kinds, or the
        ``"{eventName}:{command}"`` compound for hook kinds.

extensions/EXTENSION-API-REFERENCE.md:914

• These helpers construct identifiers; neither parses one. Directing consumers to use them for parsing is therefore not actionable and contradicts the opacity guidance. Tell consumers not to parse IDs and to retain/use the structured contribution fields instead.

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Construct and compare them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than assembling or parsing them by string-splitting on `:` — the discriminator suffix and future grammar extensions may otherwise catch you out.

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

[Copilot]

Review details

Suppressed comments (2)

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

src/specify_cli/extensions/init.py:883

• Valid colliding hooks have the same kind and synthesized name and differ only by discriminator, so this returns whichever hook appears first. Reordering those hooks changes this method's result and the other hook's ID is unreachable through this lookup API, despite the discriminator being introduced to distinguish both. Make ambiguous hook lookup explicit, such as by accepting discriminator/declared fields, looking up a full ID, or rejecting multiple matches.

        matches = []
        for entry in self.iter_contributions():
            if entry["kind"] == kind and entry.get("name") == name:

src/specify_cli/presets/init.py:5663

• ext_id here is the registry/directory key, not necessarily the loaded manifest's extension.id. _get_all_extensions_by_priority() explicitly admits unregistered directories, while _extension_manifest_declared_template() discards the manifest object after returning the entry. Thus an unregistered directory folder/ whose valid manifest declares id: actual gets extension:folder:..., but ExtensionManifest.contribution_id() returns extension:actual:..., breaking the required round-trip. Carry the manifest source ID through this resolution path, or reject/treat mismatched directory IDs as non-manifest fallbacks.

                    "lookupId": derive_named_id(
                        "extension" if entry is not None else EXTENSION_FALLBACK_LAYER,
                        ext_id,
                        template_type,
                        template_name,

• Files reviewed: 6/6 changed files
• Comments generated: 4
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (3)

src/specify_cli/presets/init.py:5663

• For a manifest-backed extension layer, the source component must come from ExtensionManifest.id, not ext_id, which here is the installation directory/registry key. Unregistered extension directories are explicitly supported, and a directory rename leaves the manifest ID unchanged but changes this lookupId; it then no longer equals ExtensionManifest.contribution_id() and violates the documented directory-move stability guarantee. Return the loaded manifest ID from _extension_manifest_declared_template() and use it when entry is present.

                    "lookupId": derive_named_id(
                        "extension" if entry is not None else EXTENSION_FALLBACK_LAYER,
                        ext_id,
                        template_type,
                        template_name,

extensions/EXTENSION-API-REFERENCE.md:865

• This statement is false for convention-discovered preset and extension layers: the resolver now emits preset-convention:... and extension-fallback:..., neither of which points to a manifest contribution. Qualify the round-trip guarantee to manifest-backed layers and document both resolver-only sentinel grammars alongside project:; otherwise consumers following the documented core|preset|extension grammar cannot interpret actual collect_all_layers() output.

Every command, template, script, and hook contributed by an extension (or a preset, or the core layer) is addressable at read time by a deterministic opaque identifier. Resolved artifact-stack layers carry a matching `lookupId` field that points back to the contribution the layer came from. Identifiers are **computed on demand from author-declared manifest content** and are **never persisted** to `.specify/` or to any cache file.

docs/reference/presets.md:223

• This round-trip claim omits convention fallbacks. collect_all_layers() now emits preset-convention:{sourceId}:{kind}:{name} and extension-fallback:{sourceId}:{kind}:{name} for undeclared files, and those intentionally match no contribution just like the project sentinel. Document these two outcomes here so the preset reference matches the resolver output.

`PresetResolver.collect_all_layers()` returns layer dicts that each include a `lookupId` field pointing back to the originating contribution's `id`. Project-local overrides in `.specify/templates/overrides/` are a resolver-only concept — they carry a synthetic `project:_:{kind}:{name}` `lookupId` that intentionally does not match any manifest contribution.

• Files reviewed: 6/6 changed files
• Comments generated: 2
• 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/extensions/init.py:803

• ExtensionManifest still accepts multiple provides.commands entries with the same canonical name, so this loop emits the same supposedly unique ID for each entry and contribution_id() silently returns the first one. The duplicate-name check only runs later in ExtensionManager._collect_manifest_command_names() (lines 1242-1316), which does not protect this new public manifest API. Reject duplicate canonical command names during manifest validation, as is already done for preset entries and extension templates/scripts.

        for cmd in self.commands:
            enriched = dict(cmd)
            name = cmd.get("name", "")
            enriched.update(
                layer="extension",

src/specify_cli/presets/init.py:5663

• For supported unregistered extension directories, ext_id is the directory name, not necessarily the loaded manifest's extension.id. _extension_manifest_declared_template() can therefore return a manifest-backed entry whose contribution ID uses the manifest ID, while this lookupId uses the directory ID and cannot round-trip. Propagate the loaded manifest ID from the helper and use it as sourceId whenever entry is present.

                    "path": candidate,
                    "source": source,
                    "strategy": "replace",
                    "extension_id": ext_id,
                    "extension_dir": ext_dir,

extensions/EXTENSION-API-REFERENCE.md:918

• The existing ## File System Layout heading was removed when this section was inserted, leaving the following .specify/ tree attached to “Opacity guidance” and eliminating the layout section. Restore the heading before the code block.

```text

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

[Copilot]

Review details

Suppressed comments (4)

src/specify_cli/presets/init.py:5638

• When entry is None, this layer came from the legacy filename convention rather than PresetManifest.iter_contributions(), so the manifest-shaped ID cannot round-trip through contribution_id(). Emit a documented non-manifest sentinel for convention-only preset layers; the fallback remains supported by the resolver at lines 5603-5604.

                        "lookupId": derive_named_id(
                            "preset", pack_id, template_type, template_name
                        ),

src/specify_cli/presets/init.py:5669

• Convention-only extension files are still resolved when no manifest entry exists (tests/test_presets.py:11934-11946). In that case this extension: ID falsely claims an originating manifest contribution that cannot exist. Use a distinct documented non-manifest sentinel whenever entry is None.

                    "lookupId": derive_named_id(
                        "extension", ext_id, template_type, template_name
                    ),

extensions/EXTENSION-API-REFERENCE.md:921

• Inserting this section replaced the existing ## File System Layout heading, leaving the .specify/ tree immediately below as an unlabelled code block. Restore that heading before the tree.

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers.

src/specify_cli/extensions/init.py:236

• Hook entries may contain arbitrary extra fields, including an authored eventName. setdefault preserves that value, so a hook under before_plan with eventName: after_plan is surfaced with eventName == after_plan while its ID and installed event use before_plan. The mapping key is authoritative; always overwrite this derived field.

        normalized.setdefault("eventName", event_name)

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

[Copilot]

Review details

Suppressed comments (5)

src/specify_cli/presets/init.py:5638

• This still emits a manifest-shaped preset ID when entry is None and resolution falls back to an undeclared conventional file. In that supported case, PresetManifest.contribution_id(...) returns None, so the advertised lookup round-trip is false. Use a documented non-manifest sentinel for convention-only preset layers and add a regression test for that path.

                        "lookupId": derive_named_id(
                            "preset", pack_id, template_type, template_name
                        ),

src/specify_cli/presets/init.py:5669

• This always derives from the directory/registry ext_id, but entry is None supports convention-only files with no manifest, and an unregistered directory can contain a valid manifest whose extension.id differs from its directory name. The first case has no contribution to join to; the second produces an ID different from ExtensionManifest.contribution_id(...), contradicting the documented directory-move stability. Use the loaded manifest ID only for declared entries and a documented non-manifest sentinel for convention fallback.

                    "lookupId": derive_named_id(
                        "extension", ext_id, template_type, template_name
                    ),

src/specify_cli/extensions/init.py:236

• setdefault preserves an author-supplied eventName even when it differs from the containing hook mapping key. The resulting contribution then reports that conflicting field while its name, id, and installed event all use event_name. Always synthesize eventName from the containing key so the contribution is internally consistent.

        normalized.setdefault("eventName", event_name)

src/specify_cli/extensions/init.py:226

• The PR description specifies discriminated IDs for duplicate hooks, canonical-JSON hashing, and rejection of byte-identical duplicates, but this implementation deliberately collapses duplicates last-write-wins and exposes no discriminator API. Update the PR identifier contract and test summary to match this compatibility behavior before merge.

def collapse_hook_event_entries(event_name: str, hook_config: Any) -> List[Dict[str, Any]]:
    """Collapse a hook event to final entries using installer last-write-wins.

    Duplicate commands are removed and re-inserted so the final declaration is
    retained at the end of the event list, matching register-time ordering.

extensions/EXTENSION-API-REFERENCE.md:921

• These helpers construct identifiers; they cannot parse an existing identifier. Telling consumers to parse with them conflicts with the opacity guidance and is not actionable. State that IDs should not be parsed, and mention the helpers only for constructing expected IDs.

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers.

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

[nicolehaugen]

@copilot for presets.md change line 218 to kind is one of command, template, or script (see below for hooks) so the reader isn't surprised.

[Copilot]

@copilot for presets.md change line 218 to kind is one of command, template, or script (see below for hooks) so the reader isn't surprised.

Updated in 739956f: line 218 now reads kind is one of command, template, or script (see below for hooks).

Posted on behalf of @nicolehaugen by GitHub Copilot (autonomous).

[Copilot]

Review details

Suppressed comments (2)

src/specify_cli/extensions/init.py:236

• A hook entry may include an extra eventName field because unknown hook fields are accepted. setdefault preserves that authored value even when it differs from the enclosing hook key, so the returned contribution can report eventName: after_plan while its name and id encode before_plan. Since the event is defined by the mapping key, always overwrite the synthesized field with event_name.

        normalized.setdefault("eventName", event_name)

extensions/EXTENSION-API-REFERENCE.md:921

• This sentence points readers to derive_named_id/derive_hook_id for parsing, but those functions only construct IDs. Also, replacing the former ## File System Layout line without restoring it leaves the following filesystem tree nested under “Opacity guidance.” Clarify that IDs must not be parsed and restore the removed heading before the tree.

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers.

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

[Copilot]

Review details

Suppressed comments (6)

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

tests/test_contribution_ids.py:109

• Issue #4210 and the PR validation summary require cross-process, relocation/reinstall, and timestamp/content-independence coverage, but this new suite only calls the derivation functions in-process and contains no subprocess, relocation, or mtime/content-change test. Add the advertised determinism tests so regressions involving ambient inputs are detectable.

class TestIdentifierDerivation:
    """Every layer x kind combination produces the expected grammar."""

src/specify_cli/presets/init.py:5639

• Convention fallback reaches this block when entry is None, so an undeclared file receives a manifest-shaped preset: ID even though PresetManifest.contribution_id() returns None. This breaks the documented lookup round-trip for a supported legacy path; use a resolver-only non-manifest sentinel and cover an undeclared conventional preset file.

                        "lookupId": derive_named_id(
                            "preset", pack_id, template_type, template_name
                        ),

src/specify_cli/presets/init.py:5673

• When no valid manifest entry exists, convention lookup still returns a supported extension layer, but this creates extension:{directory-key}:..., which cannot match any ExtensionManifest contribution. Emit a defined non-manifest sentinel for this fallback path rather than a contribution-shaped ID.

                    "lookupId": derive_named_id(
                        "extension",
                        manifest_id if entry is not None else ext_id,
                        template_type,
                        template_name,
                    ),

src/specify_cli/extensions/init.py:236

• Using setdefault lets an author-supplied eventName survive even when it differs from the enclosing hook key (or contains :). The emitted contribution then reports an eventName inconsistent with its name and id, despite the documented delimiter guarantee. Make the enclosing mapping key authoritative.

        normalized.setdefault("eventName", event_name)

src/specify_cli/extensions/init.py:226

• The current PR description still specifies discriminator hashes and rejection of byte-identical duplicate hooks, but this implementation intentionally collapses duplicates last-write-wins and has no discriminator. Update the PR contract and test summary to describe the implemented behavior before merging.

def collapse_hook_event_entries(event_name: str, hook_config: Any) -> List[Dict[str, Any]]:
    """Collapse a hook event to final entries using installer last-write-wins.

    Duplicate commands are removed and re-inserted so the final declaration is
    retained at the end of the event list, matching register-time ordering.

extensions/EXTENSION-API-REFERENCE.md:921

• These helpers construct identifiers; they do not parse an existing opaque ID. Advising consumers to parse with them is therefore not actionable and may encourage incorrect API use. State that IDs should not be parsed and reserve these helpers for constructing expected IDs.

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers.

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (7)

src/specify_cli/presets/init.py:5673

• When entry is None, this is a supported convention-only extension layer (including extensions with no manifest), so no ExtensionManifest contribution exists for the emitted extension:{ext_id}:... value. This breaks the promised lookupId round-trip. Emit a documented resolver-only sentinel for fallback layers while retaining manifest_id for declared entries.

                    "lookupId": derive_named_id(
                        "extension",
                        manifest_id if entry is not None else ext_id,
                        template_type,
                        template_name,
                    ),

src/specify_cli/presets/init.py:5639

• This always emits a manifest-shaped preset ID from the registry key. For an undeclared convention fallback, PresetManifest.contribution_id() returns None; for a manifest-backed preset whose directory/registry key differs from preset.id, it returns an ID with the manifest ID. In both cases this lookupId fails the documented join. Use manifest.id for declared entries and a documented non-manifest sentinel for convention-only layers, with regression coverage for both cases.

                        "lookupId": derive_named_id(
                            "preset", pack_id, template_type, template_name
                        ),

src/specify_cli/extensions/init.py:236

• An author-supplied eventName field can disagree with the containing hook key because setdefault preserves it. The returned contribution can then report eventName: after_plan while its name/id encode before_plan, and installation uses before_plan. Always synthesize this field from the authoritative mapping key.

        normalized.setdefault("eventName", event_name)

extensions/EXTENSION-API-REFERENCE.md:880

• The claim that extension.id is also the install-directory/registry identifier is false for supported unregistered/local-copy extensions; the new local-copy/real-id regression test demonstrates this. Clarify that manifest-backed contribution IDs use extension.id, which may differ from resolver directory or registry keys.

`sourceId` is the source's own stable system identifier: `_` for the manifest-less core layer, `preset.id` from `preset.yml` for presets, and `extension.id` from `extension.yml` for extensions. It is the same string used elsewhere to refer to that preset or extension (install directories, registries, resolver metadata, and hook metadata), which makes identifiers stable join keys back to their originating source.

docs/reference/presets.md:227

• Supported local-copy extensions can have a directory/registry key different from extension.id, as the new local-copy/real-id test shows. This row should not claim the manifest ID is always the install-directory or registry identifier.

| `extension` | The extension's `id` | The manifest's `extension.id` field — the same value used by `ExtensionManifest.id`, the install directory, registries, and hook metadata |

extensions/EXTENSION-API-REFERENCE.md:921

• These helpers construct IDs; they cannot parse an existing ID. Advising consumers to parse with them is therefore not actionable and conflicts with treating IDs as opaque. Tell consumers not to parse IDs and to compare complete values obtained from the manifest APIs.

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers.

src/specify_cli/extensions/init.py:226

• The PR description still specifies discriminator hashes and byte-identical duplicate rejection, but this helper and the tests implement last-write-wins collapse with no discriminator machinery. It also claims cross-subprocess and mtime tests that are absent from tests/test_contribution_ids.py. Update the PR contract and validation summary to match the implementation before merge.

def collapse_hook_event_entries(event_name: str, hook_config: Any) -> List[Dict[str, Any]]:
    """Collapse a hook event to final entries using installer last-write-wins.

    Duplicate commands are removed and re-inserted so the final declaration is
    retained at the end of the event list, matching register-time ordering.

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

[Copilot]

Review details

Suppressed comments (6)

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

src/specify_cli/extensions/init.py:813

• ExtensionManifest still permits duplicate primary command names until install-time validation (_collect_manifest_command_names), so this loop can emit two distinct entries with the same extension:{source}:command:{name} ID and contribution_id() then silently returns the first. Reject duplicate canonical command names during manifest validation (after name correction), as is already done for templates and scripts, and cover that manifest-level behavior.

        for cmd in self.commands:
            enriched = dict(cmd)
            name = cmd.get("name", "")
            enriched.update(
                layer="extension",
                sourceId=source_id,
                kind="command",
                id=derive_named_id("extension", source_id, "command", name),
            )

src/specify_cli/presets/init.py:5639

• When entry is None, line 5605 intentionally accepts an undeclared convention-based preset file (for example, the existing convention preset fixture in tests/test_presets.py:3654-3685). This still assigns a manifest-shaped preset ID even though PresetManifest.contribution_id() returns None, so the advertised join is false. Use a documented resolver-only sentinel for convention-discovered preset layers.

                        "lookupId": derive_named_id(
                            "preset", pack_id, template_type, template_name
                        ),

src/specify_cli/presets/init.py:5673

• The entry is None branch supports convention-only extensions, including directories with no manifest (tests/test_presets.py:11934-11946). Emitting extension:{ext_id}:... here falsely looks like a contribution ID even though no ExtensionManifest contribution exists. Use a documented non-manifest sentinel for this fallback path.

                    "lookupId": derive_named_id(
                        "extension",
                        manifest_id if entry is not None else ext_id,
                        template_type,
                        template_name,
                    ),

src/specify_cli/extensions/init.py:226

• The PR description still promises hash-discriminated duplicate hooks and rejection of byte-identical duplicates, while this implementation deliberately collapses every duplicate command last-write-wins. That is a materially different compatibility contract; update the PR identifier contract and test summary to describe the implemented collapse behavior, or restore the stated discriminator behavior.

def collapse_hook_event_entries(event_name: str, hook_config: Any) -> List[Dict[str, Any]]:
    """Collapse a hook event to final entries using installer last-write-wins.

    Duplicate commands are removed and re-inserted so the final declaration is
    retained at the end of the event list, matching register-time ordering.

extensions/EXTENSION-API-REFERENCE.md:921

• derive_named_id and derive_hook_id construct identifiers; they cannot parse an opaque identifier. This insertion also displaced the existing File System Layout heading, leaving the following directory tree under “Opacity guidance.” Describe these as construction helpers and restore the heading before the tree.

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers.

src/specify_cli/extensions/init.py:236

• setdefault preserves an author-supplied eventName that can disagree with the containing hook key. Installation and the generated name/id use event_name, but the emitted contribution then reports a different eventName; overwrite it with the actual mapping key so the contribution fields remain internally consistent.

        normalized = dict(entry)
        normalized.setdefault("eventName", event_name)

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

[Copilot]

Review details

Suppressed comments (5)

src/specify_cli/presets/init.py:5639

• Convention fallback is still reachable when entry is None, so this emits a manifest-shaped preset ID for an artifact that PresetManifest.contribution_id() cannot find. Use a distinct resolver-only sentinel for undeclared preset files (and document/test it), otherwise lookupId is a false join key.

                        "lookupId": derive_named_id(
                            "preset", pack_id, template_type, template_name
                        ),

src/specify_cli/presets/init.py:5672

• When entry is None, this layer came from the supported convention fallback, not an extension manifest contribution. The resulting extension:{directory-key}:... value therefore cannot match any ExtensionManifest ID; emit a documented fallback sentinel instead of a manifest-shaped join key.

                    "lookupId": derive_named_id(
                        "extension",
                        manifest_id if entry is not None else ext_id,
                        template_type,
                        template_name,

src/specify_cli/extensions/init.py:236

• A hook entry may contain an arbitrary eventName field because validation permits unknown fields. setdefault preserves a conflicting value even though installation and the computed ID use the outer mapping key, producing a contribution whose eventName disagrees with its name and id. Always set this derived field from event_name.

        normalized = dict(entry)
        normalized.setdefault("eventName", event_name)

src/specify_cli/extensions/init.py:226

• The current PR description specifies discriminator hashes and byte-identical duplicate rejection for repeated hooks, but this implementation deliberately collapses duplicates last-write-wins and exposes no discriminator API. Update the PR's identifier contract and test summary to match this behavior, or implement the described contract.

def collapse_hook_event_entries(event_name: str, hook_config: Any) -> List[Dict[str, Any]]:
    """Collapse a hook event to final entries using installer last-write-wins.

    Duplicate commands are removed and re-inserted so the final declaration is
    retained at the end of the event list, matching register-time ordering.

extensions/EXTENSION-API-REFERENCE.md:921

• These helpers only construct identifiers; they cannot parse an existing opaque ID. Advising consumers to parse with them is impossible and conflicts with the instruction not to interpret the string structure.

### Opacity guidance

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers.

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

[Copilot]

Review details

Suppressed comments (6)

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

tests/test_contribution_ids.py:415

• The PR description says this suite covers cross-subprocess determinism and mtime independence, but the file contains no subprocess, relocation, timestamp, or repeatability scenario; it ends with persistence checks. Add the advertised acceptance-criterion coverage or correct the PR's test summary.

class TestNoPersistence:
    def test_no_id_written_to_manifest_files(self, tmp_path):

src/specify_cli/presets/init.py:5673

• The convention fallback at lines 5654-5655 also runs when there is no valid manifest declaration, including the supported manifest-less case in tests/test_presets.py:11934-11946. Emitting extension:{ext_id}:... falsely identifies that file as a manifest contribution, so it cannot join to ExtensionManifest.contribution_id(). Use a distinct resolver-only fallback sentinel here.

                    "lookupId": derive_named_id(
                        "extension",
                        manifest_id if entry is not None else ext_id,
                        template_type,
                        template_name,
                    ),

src/specify_cli/extensions/init.py:236

• setdefault lets an arbitrary authored eventName field disagree with the enclosing hook event. For example, an entry under before_plan with eventName: after_plan is accepted and surfaced with eventName == "after_plan", while its name and id encode before_plan; an authored colon here also bypasses the new guard. Always synthesize this field from the mapping key.

        normalized.setdefault("eventName", event_name)

src/specify_cli/presets/init.py:5639

• When entry is None, this branch intentionally accepts a convention-only preset file (for example, tests/test_presets.py:12823-12839), but this value is shaped like a real preset contribution ID even though no manifest contribution exists. A reverse lookup therefore returns no match, violating the promised lookupId join. Use a documented resolver-only sentinel for undeclared preset layers and cover that round trip.

                        "lookupId": derive_named_id(
                            "preset", pack_id, template_type, template_name
                        ),

extensions/EXTENSION-API-REFERENCE.md:921

• These functions only construct identifiers; they cannot parse an existing opaque ID, so the parsing guidance is unusable. This insertion also replaced the prior ## File System Layout heading, leaving the filesystem tree immediately below inside this subsection. Advise whole-value comparison and restore that heading before the tree.

### Opacity guidance

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers.

src/specify_cli/extensions/init.py:226

• The PR description still defines discriminated IDs for distinct duplicate hooks and rejection for byte-identical duplicates, but this helper instead collapses every duplicate command with last-write-wins, and the identifier module has no discriminator implementation. Align the published contract and test summary with this behavior, or implement the advertised discriminator contract.

def collapse_hook_event_entries(event_name: str, hook_config: Any) -> List[Dict[str, Any]]:
    """Collapse a hook event to final entries using installer last-write-wins.

    Duplicate commands are removed and re-inserted so the final declaration is
    retained at the end of the event list, matching register-time ordering.

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

[Copilot]

Review details

Suppressed comments (5)

src/specify_cli/presets/init.py:5639

• Convention fallback reaches this block when the preset does not declare the artifact, but this still emits a manifest-shaped preset: ID. In that case PresetManifest.contribution_id() returns None, so the advertised lookup round-trip is false. Use a documented resolver-only sentinel for undeclared preset files and cover that fallback explicitly.

                        "lookupId": derive_named_id(
                            "preset", pack_id, template_type, template_name
                        ),

src/specify_cli/presets/init.py:5673

• When entry is None, this is the supported convention-only extension path (including extensions with no manifest), yet it emits an extension: ID that cannot match any ExtensionManifest contribution. Use a distinct resolver-only fallback sentinel rather than a manifest contribution ID.

                    "lookupId": derive_named_id(
                        "extension",
                        manifest_id if entry is not None else ext_id,
                        template_type,
                        template_name,
                    ),

src/specify_cli/extensions/init.py:236

• An accepted hook entry may already contain an eventName field different from its enclosing mapping key. setdefault preserves that value, so the returned contribution can say eventName: after_plan while its name and id are derived from before_plan. Always synthesize this field from the authoritative mapping key.

        normalized.setdefault("eventName", event_name)

extensions/EXTENSION-API-REFERENCE.md:921

• This section now runs directly into the filesystem tree without restoring the removed File System Layout heading. It also tells consumers to parse IDs with derive_*, but those helpers only construct IDs and cannot parse them. Restore the heading and describe the helpers as constructors while keeping IDs opaque.

### Opacity guidance

Identifiers are stable, but treat them as **opaque strings** in stored data (registries, cache files, external tooling). Parse them with the helpers in `specify_cli._identifier` (`derive_named_id`, `derive_hook_id`) rather than by string-splitting on `:` so future grammar extensions do not break consumers.

src/specify_cli/extensions/init.py:226

• The PR description still promises discriminator hashes, canonical-JSON derivation, and byte-identical duplicate rejection, while this implementation intentionally collapses duplicate hook commands last-write-wins and exposes no discriminator API. Update the identifier contract and test summary in the PR body to match this compatibility behavior.

def collapse_hook_event_entries(event_name: str, hook_config: Any) -> List[Dict[str, Any]]:
    """Collapse a hook event to final entries using installer last-write-wins.

    Duplicate commands are removed and re-inserted so the final declaration is
    retained at the end of the event list, matching register-time ordering.

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

[nicolehaugen]

Superseded by #4305, which consolidates this work with #4267 into a single PR against main. The identifier grammar here and the resolver changes in #4267 evolved together through review — splitting them cleanly would leave both PRs with incomplete stories, so ship as one.