fix: preserve colon-containing hook artifacts

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

Author: nicolehaugen

docs/reference/artifacts.md

@@ -154,7 +154,7 @@ Lookup IDs are derived by the artifact command from the resolved layer and its e
 Hook rows project hook declarations from extensions included by the standard preset and extension resolver. The round-trip shorthand is:
 
 ```text
-hook:{eventName}:{targetCommand}
+hook:{encodedEventName}:{encodedTargetCommand}
 ```
 
 For example, both of these select the same hook artifact:
@@ -164,6 +164,12 @@ specify artifact info hook:before_specify:speckit.compliance.pre-check --json
 specify artifact info before_specify:speckit.compliance.pre-check --kind hook --json
 ```
 
+Hook ID components are percent-encoded only when required to keep the colon-delimited shorthand unambiguous. This encoding is limited to the artifact `id`, `name`, and `lookupId` representation; hook manifests, runtime bindings, `eventName`, and `targetCommand` are unchanged. For example, an event named `custom:after` targeting `/skill:speckit-test-ext-hello` has the artifact ID:
+
+```text
+hook:custom%3Aafter:%2Fskill%3Aspeckit-test-ext-hello
+```
+
 Hook rows add three top-level fields:
 
 | Field           | Description                                                                    |
@@ -178,7 +184,7 @@ For hooks, `active` reports registration state from `.specify/extensions.yml`, n
 
 Declared-but-unregistered hooks remain visible when their extension is included by the normal resolver. Registry-disabled extensions are excluded entirely, consistently with their other contributions. Invalid individual extension manifests are also omitted by the existing resolver and remain diagnosable through extension inspection and validation commands.
 
-Hook lookup IDs use the artifact-private `{layer}:{sourceId}:hook:{eventName}:{targetCommand}` grammar. Hook provenance is restricted to `preset` and `extension` layers; hooks never receive a built-in/core layer. The current manifest API exposes extension hook declarations, so current rows use the `extension` layer. The `preset` layer remains reserved by the hook identifier grammar for preset-provided hooks without requiring artifact IDs to be added to preset or extension manifest APIs.
+Hook lookup IDs use the artifact-private `{layer}:{sourceId}:hook:{encodedEventName}:{encodedTargetCommand}` grammar. Hook provenance is restricted to `preset` and `extension` layers; hooks never receive a built-in/core layer. The current manifest API exposes extension hook declarations, so current rows use the `extension` layer. The `preset` layer remains reserved by the hook identifier grammar for preset-provided hooks without requiring artifact IDs to be added to preset or extension manifest APIs.
 
 ## JSON Errors
 

src/specify_cli/artifacts/_identifiers.py

@@ -2,12 +2,15 @@
 
 from __future__ import annotations
 
+import re
 from typing import Any
+from urllib.parse import quote, unquote_to_bytes
 
 PROJECT_OVERRIDE_LAYER = "project"
 _ARTIFACT_KINDS = frozenset({"command", "template", "script"})
 _LAYER_KINDS = frozenset({PROJECT_OVERRIDE_LAYER, "preset", "extension"})
 _HOOK_LAYERS = frozenset({"preset", "extension"})
+_INVALID_PERCENT_ESCAPE = re.compile(r"%(?![0-9A-Fa-f]{2})")
 
 
 class IdentifierComponentError(ValueError):
@@ -63,9 +66,9 @@ def derive_lookup_id(layer: str, source_id: str, kind: str, name: str) -> str:
 
 def derive_hook_public_id(event_name: str, command: str) -> str:
     """Build the source-agnostic identifier for a hook artifact."""
-    validate_component(event_name, "eventName")
-    validate_component(command, "command")
-    return f"hook:{event_name}:{command}"
+    encoded_event = _encode_hook_component(event_name, "eventName")
+    encoded_command = _encode_hook_component(command, "command")
+    return f"hook:{encoded_event}:{encoded_command}"
 
 
 def derive_hook_lookup_id(
@@ -74,12 +77,56 @@ def derive_hook_lookup_id(
     """Build the artifact-private lookup identifier for a hook declaration."""
     validate_component(layer, "layer")
     validate_component(source_id, "sourceId")
-    validate_component(event_name, "eventName")
-    validate_component(command, "command")
+    encoded_event = _encode_hook_component(event_name, "eventName")
+    encoded_command = _encode_hook_component(command, "command")
     if layer not in _HOOK_LAYERS:
         raise IdentifierComponentError(f"Invalid hook layer '{layer}'")
     if source_id == "_":
         raise IdentifierComponentError(
             "Invalid sourceId '_': hooks require a preset or extension source"
         )
-    return f"{layer}:{source_id}:hook:{event_name}:{command}"
+    return f"{layer}:{source_id}:hook:{encoded_event}:{encoded_command}"
+
+
+def parse_hook_artifact_name(name: str) -> tuple[str, str]:
+    """Decode the ``{eventName}:{targetCommand}`` portion of a hook artifact ID."""
+    encoded_event, separator, encoded_command = name.partition(":")
+    if not separator or ":" in encoded_command:
+        raise IdentifierComponentError("Invalid hook artifact name")
+    return (
+        _decode_hook_component(encoded_event, "eventName"),
+        _decode_hook_component(encoded_command, "command"),
+    )
+
+
+def _encode_hook_component(value: Any, field_label: str) -> str:
+    """Encode one hook ID component without narrowing manifest syntax."""
+    if not isinstance(value, str):
+        raise IdentifierComponentError(
+            f"Invalid {field_label}: expected a string, got {type(value).__name__}"
+        )
+    if not value:
+        raise IdentifierComponentError(
+            f"Invalid {field_label}: value must not be empty"
+        )
+    return quote(value, safe="")
+
+
+def _decode_hook_component(value: str, field_label: str) -> str:
+    """Decode one hook ID component, rejecting malformed percent escapes."""
+    validate_component(value, field_label)
+    if _INVALID_PERCENT_ESCAPE.search(value):
+        raise IdentifierComponentError(
+            f"Invalid {field_label}: malformed percent escape"
+        )
+    try:
+        decoded = unquote_to_bytes(value).decode("utf-8")
+    except UnicodeDecodeError as exc:
+        raise IdentifierComponentError(
+            f"Invalid {field_label}: value is not valid UTF-8"
+        ) from exc
+    if not decoded:
+        raise IdentifierComponentError(
+            f"Invalid {field_label}: value must not be empty"
+        )
+    return decoded

src/specify_cli/artifacts/catalog.py

@@ -24,6 +24,7 @@
     derive_hook_lookup_id,
     derive_hook_public_id,
     derive_public_id,
+    parse_hook_artifact_name,
     validate_component,
 )
 from .models import (
@@ -240,11 +241,8 @@ def _resolve_kind_hint(name: str, kind: ArtifactKind | None) -> tuple[str, Artif
 def _validate_artifact_name(name: str, kind: ArtifactKind) -> str:
     """Validate the structural identifier component constraints for ``name``."""
     if kind == "hook":
-        event_name, separator, command = name.partition(":")
-        if not separator:
-            raise ArtifactNotFoundError(name)
         try:
-            derive_hook_public_id(event_name, command)
+            parse_hook_artifact_name(name)
         except IdentifierComponentError as exc:
             raise ArtifactNotFoundError(name) from exc
         return name
@@ -403,7 +401,7 @@ def _get_hook_info(
     ) -> dict[str, Any]:
         """Return one hook row and its additive declaration stack."""
         _validate_artifact_name(bare_name, "hook")
-        event_name, _, command = bare_name.partition(":")
+        event_name, command = parse_hook_artifact_name(bare_name)
         hook_rows, stack_cache = self._collect_hook_inventory(resolver)
         for row in hook_rows:
             if row.eventName != event_name or row.targetCommand != command:
@@ -473,23 +471,29 @@ def _collect_hook_inventory(
                 source_id = manifest.id
 
                 for event_name, hook_config in (manifest.hooks or {}).items():
-                    entries_by_command: dict[str, dict[str, Any]] = {}
+                    entries_by_command: dict[
+                        str, tuple[dict[str, Any], str, str]
+                    ] = {}
                     for entry in coerce_hook_entries(hook_config):
                         if not isinstance(entry, dict):
                             continue
                         command = entry.get("command")
                         try:
-                            validate_component(command, "command")
+                            public_id = derive_hook_public_id(event_name, command)
+                            lookup_id = derive_hook_lookup_id(
+                                "extension", source_id, event_name, command
+                            )
                         except IdentifierComponentError:
                             continue
                         entries_by_command.pop(command, None)
-                        entries_by_command[command] = entry
-
-                    for command, entry in entries_by_command.items():
-                        public_id = derive_hook_public_id(event_name, command)
-                        lookup_id = derive_hook_lookup_id(
-                            "extension", source_id, event_name, command
+                        entries_by_command[command] = (
+                            entry,
+                            public_id,
+                            lookup_id,
                         )
+
+                    for command, entry_data in entries_by_command.items():
+                        entry, public_id, lookup_id = entry_data
                         insertion_index += 1
                         declaration = {
                             "id": public_id,
@@ -560,10 +564,11 @@ def _collect_hook_inventory(
                 ),
                 "",
             )
+            public_id = derive_hook_public_id(event_name, command)
             rows.append(
                 HookArtifact(
-                    id=derive_hook_public_id(event_name, command),
-                    name=f"{event_name}:{command}",
+                    id=public_id,
+                    name=public_id.removeprefix("hook:"),
                     kind="hook",
                     description=description,
                     eventName=event_name,

tests/test_artifact_command.py

@@ -1948,6 +1948,42 @@ def test_hook_shorthand_round_trips(self, spec_kit_project: Path):
         assert payload["eventName"] == "before_specify"
         assert payload["targetCommand"] == "speckit.compliance.pre-check"
 
+    def test_colon_containing_values_round_trip_through_encoded_id(
+        self, spec_kit_project: Path
+    ):
+        _install_extension_with_hooks(
+            spec_kit_project,
+            "ext",
+            hooks={
+                "custom:after": [
+                    {
+                        "command": "/skill:speckit-test-ext-hello",
+                        "description": "Colon-compatible hook",
+                    }
+                ]
+            },
+        )
+
+        rows = ArtifactCatalog(spec_kit_project).list_artifacts_with_stack()
+        row = next(item for item in rows if item["kind"] == "hook")
+
+        assert (
+            row["id"]
+            == "hook:custom%3Aafter:%2Fskill%3Aspeckit-test-ext-hello"
+        )
+        assert row["name"] == (
+            "custom%3Aafter:%2Fskill%3Aspeckit-test-ext-hello"
+        )
+        assert row["eventName"] == "custom:after"
+        assert row["targetCommand"] == "/skill:speckit-test-ext-hello"
+        assert row["stack"][0]["lookupId"] == (
+            "extension:ext:hook:custom%3Aafter:"
+            "%2Fskill%3Aspeckit-test-ext-hello"
+        )
+
+        info = ArtifactCatalog(spec_kit_project).get_artifact_info(row["id"])
+        assert info == row
+
     def test_kind_hint_resolves_hook_name(self, spec_kit_project: Path):
         _install_extension_with_hooks(
             spec_kit_project,