From 8e828dcdaca0a5079faf3cdc0e44dc89f69c9d64 Mon Sep 17 00:00:00 2001 From: kartsan03 Date: Tue, 27 Oct 2026 20:52:39 +0000 Subject: [PATCH 1/2] fix(extensions): reject hook commands that are not strings `ExtensionManifest` only checked that a hook's `command` was truthy, so `command: [speckit.x.hello]` or a mapping passed validation. `extension add` then crashed in `HookExecutor.register_hooks` with `TypeError: unhashable type: 'list'`, after the extension directory and the agent skill had already been written. Apply the rule extension events already use (#17): a hook command must be a non-blank string, so the manifest is rejected with a `ValidationError` before anything is installed. Refs #5080 Assisted-by: Codex CLI (model: gpt-6.1-sol, autonomous) Assisted-by: Claude Code (model: claude-opus-5-5, autonomous) --- src/specify_cli/extensions/__init__.py | 8 ++++-- .../extensions/test_command_add.py | 28 +++++++++++++++++++ tests/test_extensions.py | 20 +++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index c22445ebe0..451e0c83db 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -432,9 +432,13 @@ def _validate(self): f"Invalid hook '{hook_name}': " "expected a mapping or list of mappings" ) - if not entry.get("command"): + command = entry.get("command") + # #5080: same rule as extension events (#17). A truthy + # non-string (e.g. command: [foo]) passed a bare + # truthiness check and crashed hook registration mid-install. + if not isinstance(command, str) or not command.strip(): raise ValidationError( - f"Hook '{hook_name}' missing required 'command' field" + f"Hook '{hook_name}' missing required 'command' string" ) if "priority" in entry: priority = entry["priority"] diff --git a/tests/specify_cli/extensions/test_command_add.py b/tests/specify_cli/extensions/test_command_add.py index 44423b5fdc..f898fa052f 100644 --- a/tests/specify_cli/extensions/test_command_add.py +++ b/tests/specify_cli/extensions/test_command_add.py @@ -40,6 +40,34 @@ class TestExtensionAddCLI: """CLI tests for ``specify extension add``.""" + @pytest.mark.parametrize("command", [ + pytest.param(["speckit.test-ext.hello"], id="list"), + pytest.param({"name": "speckit.test-ext.hello"}, id="mapping"), + ]) + def test_add_dev_rejects_invalid_hook_command_before_writing_files( + self, extension_dir, project_dir, command + ): + """Malformed hook commands must leave no installed artifacts (#5080).""" + manifest_path = extension_dir / "extension.yml" + data = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + data["hooks"]["after_tasks"]["command"] = command + manifest_path.write_text(yaml.safe_dump(data), encoding="utf-8") + agent_dir = project_dir / ".claude" + (agent_dir / "skills").mkdir(parents=True) + before = set(agent_dir.rglob("*")) + + with patch.object(Path, "cwd", return_value=project_dir): + result = CliRunner().invoke(app, ["extension", "add", "--dev", str(extension_dir)]) + + assert result.exit_code == 1, result.output + assert isinstance(result.exception, SystemExit), repr(result.exception) + output = strip_ansi(result.output) + assert "after_tasks" in output and "'command'" in output, output + assert "Traceback" not in output + assert not (project_dir / ".specify/extensions/test-ext").exists() + assert set(agent_dir.rglob("*")) == before + assert ExtensionRegistry(project_dir / ".specify/extensions").get("test-ext") is None + def test_add_dev_links_copilot_agent_when_supported( self, extension_dir, project_dir, temp_dir ): diff --git a/tests/test_extensions.py b/tests/test_extensions.py index f2a75d08a4..07142e9c24 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -878,6 +878,26 @@ def test_non_dict_hook_entry_raises_validation_error(self, temp_dir, valid_manif with pytest.raises(ValidationError, match="Invalid hook 'after_tasks'"): ExtensionManifest(manifest_path) + @pytest.mark.parametrize("as_list", [False, True]) + @pytest.mark.parametrize("command", [ + pytest.param(["speckit.test-ext.hello"], id="list"), + pytest.param({"name": "speckit.test-ext.hello"}, id="mapping"), + pytest.param(123, id="integer"), + pytest.param(True, id="boolean"), + pytest.param(" ", id="blank"), + ]) + def test_hook_command_must_be_a_string( + self, temp_dir, valid_manifest_data, command, as_list + ): + """Reject non-string commands before hook registration can fail (#5080).""" + entry = {"command": command} + valid_manifest_data["hooks"]["after_tasks"] = [entry] if as_list else entry + manifest_path = temp_dir / "extension.yml" + manifest_path.write_text(yaml.safe_dump(valid_manifest_data), encoding="utf-8") + + with pytest.raises(ValidationError, match="Hook 'after_tasks' missing required 'command' string"): + ExtensionManifest(manifest_path) + def test_hook_single_mapping_still_accepted(self, extension_dir): """Existing single-mapping hook manifests parse unchanged (regression).""" manifest_path = extension_dir / "extension.yml" From b6d14f334185f5dc2b3d6eb30824acd250393c73 Mon Sep 17 00:00:00 2001 From: kartsan03 Date: Wed, 28 Oct 2026 15:07:23 +0000 Subject: [PATCH 2/2] fix(extensions): point the hook command comment at validate_events() `#17` read as a GitHub issue reference, but that issue is about Gemini CLI. Refs #5080 Assisted-by: Claude Code (model: claude-opus-5-5, autonomous) --- src/specify_cli/extensions/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 451e0c83db..dabde70bbc 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -433,8 +433,8 @@ def _validate(self): "expected a mapping or list of mappings" ) command = entry.get("command") - # #5080: same rule as extension events (#17). A truthy - # non-string (e.g. command: [foo]) passed a bare + # #5080: same rule as validate_events() in events/__init__.py. + # A truthy non-string (e.g. command: [foo]) passed a bare # truthiness check and crashed hook registration mid-install. if not isinstance(command, str) or not command.strip(): raise ValidationError(