fix(cli): harden extension registration and discovery workflows by DyanGalih · Pull Request #2499 · github/spec-kit

DyanGalih · 2026-05-08T10:41:19Z

This PR hardens the extension registration process in the Spec Kit CLI and updates governance orchestration workflows to ensure robust extension discovery.

Key Changes

CLI Hardening: Updated HookExecutor and ExtensionManager to automatically maintain an installed list in .specify/extensions.yml.
Detection Logic: Orchestration templates now prioritize the installed list for discovery.
Unit Tests: Added comprehensive tests in tests/test_extension_registration.py.

- Update memory-md from 0.7.9 to 0.8.0 - Update architecture-guard from 1.6.7 to 1.8.0

…n extensions.yml

Copilot

Pull request overview

This PR strengthens how the Spec Kit CLI tracks installed extensions by ensuring hook registration/unregistration updates an installed list in .specify/extensions.yml, and adds tests to validate the intended behavior. It also updates metadata for a couple of community extensions in the catalog.

Changes:

Add installed list maintenance to HookExecutor (register on hook registration; unregister on hook removal).
Introduce unit tests covering registration sorting/idempotency and initialization when installed is missing.
Bump versions + download URLs (and updated_at) for two entries in extensions/catalog.community.json.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`tests/test_extension_registration.py`	Adds new unit tests validating `installed` list behavior in `.specify/extensions.yml`.
`src/specify_cli/extensions.py`	Updates hook registration/unregistration to maintain an `installed` extension list; adds helper methods.
`extensions/catalog.community.json`	Updates version/download metadata for `architecture-guard` and `memory-md`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

DyanGalih · 2026-05-08T11:44:11Z

+        config = self.get_project_config()
+
+        if "installed" not in config:
+            config["installed"] = []
+
+        if extension_id not in config["installed"]:
+            config["installed"].append(extension_id)
+            # Maintain alphabetical order for readability and diff stability
+            config["installed"].sort()
+            self.save_project_config(config)


✅ Fixed in commit 019acae.

The code now normalizes both the config and the installed field before any mutation:

config = self.get_project_config() # Ensure config is a dict (defensive) if not isinstance(config, dict): config = {} # Ensure "installed" is a list (defensive) if "installed" not in config or not isinstance(config["installed"], list): config["installed"] = [] if extension_id not in config["installed"]: config["installed"].append(extension_id) config["installed"].sort() self.save_project_config(config)

Handles all edge cases: corrupted root (list/string) → {}, missing/null/non-list installed → []. The in check, .append(), and .sort() are all safe.

DyanGalih · 2026-05-08T11:44:12Z

+        # Register hooks and update installed list in extensions.yml
        hook_executor = HookExecutor(self.project_root)
        hook_executor.register_hooks(manifest)


Good catch. The PR description was written to reflect the full intended scope. The orchestration template changes are handled in a companion change in the spec-kit-architecture-guard extension. This PR focuses solely on the CLI hardening.

I'll update the PR description to clarify this separation to avoid confusion.

DyanGalih · 2026-05-08T10:49:13Z

Addressing feedback:

Defensive Logic: Normalizing config/list types in and to handle corrupted YAML.
Cleanup Flow: Moved to the start of to ensure cleanup even if hooks are missing.
Template Clarification: The orchestration templates reside in and have been updated in a companion change to match this CLI hardening.
Clean Code: Removed unused import in tests.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src/specify_cli/extensions.py:2562

register_hooks() now always calls register_extension(), but the subsequent hook registration path still assumes config is a dict and that config["hooks"] is a dict. Since get_project_config() can return any YAML type and existing configs could have a non-mapping hooks value, consider hardening here too (e.g., return defaults/coerce when config or config["hooks"] isn’t a mapping) so installs don’t crash on corrupted extensions.yml.

        # Always ensure the extension is in the installed list
        self.register_extension(manifest.id)

        if not hasattr(manifest, "hooks") or not manifest.hooks:
            return

        config = self.get_project_config()

        # Ensure hooks dict exists
        if "hooks" not in config:
            config["hooks"] = {}

DyanGalih · 2026-05-08T11:44:13Z

+        # Ensure "installed" is a list (defensive)
+        if "installed" not in config or not isinstance(config["installed"], list):
+            config["installed"] = []
+
+        if extension_id not in config["installed"]:
+            config["installed"].append(extension_id)
+            # Maintain alphabetical order for readability and diff stability
+            config["installed"].sort()
+            self.save_project_config(config)


✅ Fixed in commit 019acae.

The installed list is now coerced to [] when it is missing, null, or any non-list type — before any membership check or .sort() call. No TypeError can occur from non-string entries in a corrupted list.

DyanGalih · 2026-05-08T11:44:13Z

+        self.unregister_extension(extension_id)
+
        config = self.get_project_config()



✅ Fixed in commit 019acae.

unregister_hooks() now has an early isinstance(config, dict) guard:

config = self.get_project_config() if "hooks" not in config or not isinstance(config["hooks"], dict): return

If the root config is a list or scalar, the method returns early without raising.

DyanGalih · 2026-05-08T11:44:14Z

            config["hooks"][hook_name] = [
                h
                for h in config["hooks"][hook_name]


✅ Fixed in commit 019acae.

The list comprehension now guards with isinstance(h, dict) before calling .get():

config["hooks"][hook_name] = [ h for h in config["hooks"][hook_name] if isinstance(h, dict) and h.get("extension") != extension_id ]

Non-dict values (null, scalars) are silently skipped rather than raising TypeError.

DyanGalih · 2026-05-08T11:44:15Z

+
+        manifest = ExtensionManifest(manifest_path)
+        executor = HookExecutor(project_dir)
+
+        # This should call register_extension internally
+        executor.register_hooks(manifest)
+
+        config = executor.get_project_config()
+        assert "hook-ext" in config["installed"]
+
+    def test_missing_installed_key_initialization(self, project_dir):
+        """Graceful Initialization: If 'installed' key is missing, it should be created."""
+        executor = HookExecutor(project_dir)
+
+        # Manually create a config without 'installed'
+        config_path = project_dir / ".specify" / "extensions.yml"
+        config_path.write_text(yaml.dump({"settings": {"auto_execute_hooks": True}}))
+
+        # This should detect the missing key and initialize it
+        executor.register_extension("new-ext")
+


✅ Fixed in commit 019acae.

Added the following integration tests covering both workflows:

test_register_hooks_triggers_registration — register_hooks() with a manifest that has hooks, verifies installed is updated

test_unregister_hooks_full_workflow — full removal: hooks removed + installed pruned

test_unregister_hooks_no_hooks_key — unregister_hooks() with no hooks key still prunes installed

test_unregister_hooks_corrupted_config — graceful handling of corrupted YAML

test_unregister_hooks_with_multiple_extensions — only target extension is removed

All 11 tests passing. ✅

…ive unregister_hooks tests - Add dict guard to register_hooks() to handle corrupted extensions.yml (non-dict root) - Add 5 comprehensive tests for unregister_hooks() workflow: * Full workflow with hooks + installed list removal * Resilience when config has no 'hooks' key * Corrupted YAML handling * Multiple extension scenarios * All 11 tests passing

DyanGalih

All feedback incorporated. Ready for approval! 🎉

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

DyanGalih · 2026-05-08T11:44:15Z

        # Ensure hooks dict exists
        if "hooks" not in config:


✅ Fixed in commit 019acae.

register_hooks() now normalizes the config before touching hooks:

config = self.get_project_config() # Ensure config is a dict (defensive) if not isinstance(config, dict): config = {} # Ensure hooks dict exists if "hooks" not in config or not isinstance(config["hooks"], dict): config["hooks"] = {}

Both missing and non-dict hooks values (e.g., hooks: []) are reset to {} before any key access.

DyanGalih · 2026-05-08T11:44:16Z

        config = self.get_project_config()

-        if "hooks" not in config:
+        if "hooks" not in config or not isinstance(config["hooks"], dict):
            return

        # Remove hooks for this extension
-        for hook_name in config["hooks"]:
+        for hook_name in list(config["hooks"].keys()):
            config["hooks"][hook_name] = [
                h
                for h in config["hooks"][hook_name]
-                if h.get("extension") != extension_id
+                if isinstance(h, dict) and h.get("extension") != extension_id
            ]


✅ Fixed in commit 019acae.

unregister_hooks() now has both guards in place:

self.unregister_extension(extension_id) # always runs first config = self.get_project_config() if "hooks" not in config or not isinstance(config["hooks"], dict): return # safe for non-dict root or missing/non-dict hooks

And each hook list is filtered with isinstance(h, dict) to handle null/scalar entries:

config["hooks"][hook_name] = [ h for h in config["hooks"][hook_name] if isinstance(h, dict) and h.get("extension") != extension_id ]

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

DyanGalih · 2026-05-08T12:09:22Z

+        # Ensure "installed" is a list (defensive)
+        if "installed" not in config or not isinstance(config["installed"], list):
+            config["installed"] = []
+
+        if extension_id not in config["installed"]:
+            config["installed"].append(extension_id)
+            # Maintain alphabetical order for readability and diff stability
+            config["installed"].sort()
+            self.save_project_config(config)


✅ Fixed in commit 51d19ce.

The installed list is now sanitized to strings-only before any membership check or sort:

if "installed" not in config or not isinstance(config["installed"], list): config["installed"] = [] else: # Sanitize: keep only strings to prevent TypeError on sort config["installed"] = [x for x in config["installed"] if isinstance(x, str)]

Non-string entries (e.g., 1, True) are silently dropped before .sort() is called. Regression test test_register_extension_mixed_type_installed verifies installed: [1, True, "existing-ext"] is handled without TypeError.

DyanGalih · 2026-05-08T12:09:32Z

+        # Ensure config is a dict (defensive)
+        if not isinstance(config, dict):
+            config = {}
+
        # Ensure hooks dict exists
        if "hooks" not in config:
            config["hooks"] = {}


✅ Fixed in commit 51d19ce.

register_hooks() now normalizes hooks whether it is missing or non-dict:

# Ensure hooks dict exists and is a mapping if "hooks" not in config or not isinstance(config["hooks"], dict): config["hooks"] = {}

hooks: [] or hooks: null are reset to {} before any key assignment.

DyanGalih · 2026-05-08T12:09:43Z

+        # Always remove from installed list (Feedback from review)
+        self.unregister_extension(extension_id)
+
        config = self.get_project_config()

-        if "hooks" not in config:
+        if "hooks" not in config or not isinstance(config["hooks"], dict):
            return


✅ Fixed in commit 51d19ce.

unregister_hooks() now has both an isinstance(config, dict) guard and coerces non-list hook values:

if not isinstance(config, dict): return if "hooks" not in config or not isinstance(config["hooks"], dict): return for hook_name in list(config["hooks"].keys()): hook_list = config["hooks"][hook_name] if not isinstance(hook_list, list): config["hooks"][hook_name] = [] continue config["hooks"][hook_name] = [ h for h in hook_list if isinstance(h, dict) and h.get("extension") != extension_id ]

A scalar config (123) returns early. A null/non-list hook value is reset to [] instead of raising TypeError.

DyanGalih · 2026-05-08T12:09:53Z

+        # Register hooks and update installed list in extensions.yml
        hook_executor = HookExecutor(self.project_root)
        hook_executor.register_hooks(manifest)


Agreed. Updated the PR description to clarify this PR is scoped to CLI hardening only. Orchestration template updates that consume the installed list are tracked as a companion change in spec-kit-architecture-guard.

DyanGalih · 2026-05-08T12:10:04Z

+
+        config = executor.get_project_config()
+        assert "hook-ext" in config["installed"]
+


✅ Fixed in commit 51d19ce.

Added test_register_hooks_no_hooks_still_registers — uses a commands-only manifest (no hooks key) and asserts the extension ID is still persisted in installed after register_hooks():

executor.register_hooks(manifest) # manifest has no hooks config = executor.get_project_config() assert "commands-only-ext" in config["installed"]

All 14 tests passing. ✅

DyanGalih · 2026-05-08T12:10:17Z

+        config = executor.get_project_config()
+        assert "installed" in config
+        assert config["installed"] == ["new-ext"]
+


✅ Fixed in commit 51d19ce.

Added two targeted regression tests:

test_register_extension_mixed_type_installed — writes installed: [1, True, "existing-ext"] and verifies no TypeError, non-strings are dropped, valid strings are kept.

test_unregister_hooks_null_hook_values — writes hooks: {after_tasks: null} and verifies unregister_hooks() completes without TypeError, and the extension is removed from installed.

All 14 tests passing. ✅

…le null hook values - register_extension(): filter non-string entries from installed before sort - register_hooks(): normalize hooks to {} when missing or not a dict - unregister_hooks(): add isinstance(config, dict) guard before key checks - unregister_hooks(): coerce null/scalar hook lists to [] before iteration - tests: add 3 regression tests for no-hooks manifest, mixed-type installed, null hook values - All 14 tests passing

DyanGalih added 3 commits May 7, 2026 22:59

chore: update community catalog with latest extension versions

0b244a8

- Update memory-md from 0.7.9 to 0.8.0 - Update architecture-guard from 1.6.7 to 1.8.0

fix(cli): harden extension registration with project-level tracking i…

730918e

…n extensions.yml

test(cli): add comprehensive unit tests for extension registration logic

c953140

Copilot AI review requested due to automatic review settings May 8, 2026 10:41

DyanGalih requested a review from mnriem as a code owner May 8, 2026 10:41

Copilot started reviewing on behalf of DyanGalih May 8, 2026 10:41 View session

chore: remove out-of-scope catalog changes

4af789a

Copilot AI reviewed May 8, 2026

View reviewed changes

refactor: address PR feedback for extension registration hardening

0fd9a14

DyanGalih requested a review from Copilot May 8, 2026 11:11

Copilot started reviewing on behalf of DyanGalih May 8, 2026 11:12 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

DyanGalih requested a review from Copilot May 8, 2026 11:29

Copilot started reviewing on behalf of DyanGalih May 8, 2026 11:30 View session

DyanGalih commented May 8, 2026

View reviewed changes

Copilot AI reviewed May 8, 2026

View reviewed changes

DyanGalih requested a review from Copilot May 8, 2026 11:45

Copilot started reviewing on behalf of DyanGalih May 8, 2026 11:46 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

		self.unregister_extension(extension_id)

		config = self.get_project_config()


		config = executor.get_project_config()
		assert "hook-ext" in config["installed"]

Conversation

DyanGalih commented May 8, 2026

Key Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DyanGalih commented May 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DyanGalih left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DyanGalih left a comment •

edited

Loading