From 4666a959e6c22989c20bee56f61026ece6408e1d Mon Sep 17 00:00:00 2001 From: joshinitin2016883-sketch Date: Wed, 28 Oct 2026 22:20:41 +0530 Subject: [PATCH 1/2] fix(extensions): load top-level defaults from extension.yml ConfigManager._get_extension_defaults read only `config.defaults`, while the extension template, EXTENSION-API-REFERENCE.md and EXTENSION-DEVELOPMENT-GUIDE.md document a top-level `defaults:` key. Defaults declared the documented way were silently dropped, so hook conditions such as `config.feature.enabled == 'true'` treated them as unset. Read both keys and merge them, with `config.defaults` winning on conflicting keys, so manifests that use it (including the bundled git extension) resolve exactly as before. A non-mapping value under either key is ignored, as a malformed `config` section already is. Update the precedence list in design/extension.md to match. Fixes #5084 Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous) Co-Authored-By: Claude Opus 5.5 --- design/extension.md | 3 +- src/specify_cli/extensions/__init__.py | 14 +++++- tests/test_extensions.py | 60 ++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/design/extension.md b/design/extension.md index 8eb6a29199..fea066f57a 100644 --- a/design/extension.md +++ b/design/extension.md @@ -141,7 +141,8 @@ Catalog configuration resolves in this order: Extension configuration merges, from lowest to highest precedence: -1. `config.defaults` in `extension.yml`; +1. the top-level `defaults` and `config.defaults` in `extension.yml`, merged + with `config.defaults` winning on conflicting keys; 2. `.specify/extensions//-config.yml`; 3. `.specify/extensions//local-config.yml`; and 4. `SPECKIT__*` environment variables. diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index dabde70bbc..f6005f6012 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -5308,6 +5308,11 @@ def _load_yaml_config(self, file_path: Path) -> Dict[str, Any]: def _get_extension_defaults(self) -> Dict[str, Any]: """Get default configuration from extension manifest. + Reads the top-level ``defaults`` key documented in the extension + template and API reference, and ``config.defaults`` used by the + bundled git extension. When both are present they are merged, with + ``config.defaults`` winning on conflicting keys. + Returns: Default configuration dictionary """ @@ -5316,6 +5321,9 @@ def _get_extension_defaults(self) -> Dict[str, Any]: return {} manifest_data = self._load_yaml_config(manifest_path) + top_level_defaults = manifest_data.get("defaults", {}) + if not isinstance(top_level_defaults, dict): + top_level_defaults = {} # _load_yaml_config already coerces a non-mapping *root* to {}, but # extension.yml's top-level 'config' key is unvalidated by # ExtensionManifest (only 'provides.config' is checked there -- a @@ -5332,9 +5340,11 @@ def _get_extension_defaults(self) -> Dict[str, Any]: # deeper in the manifest's own 'config' section. config_section = manifest_data.get("config", {}) if not isinstance(config_section, dict): - return {} + return top_level_defaults defaults = config_section.get("defaults", {}) - return defaults if isinstance(defaults, dict) else {} + if not isinstance(defaults, dict): + return top_level_defaults + return self._merge_configs(top_level_defaults, defaults) def _get_project_config(self) -> Dict[str, Any]: """Get project-level configuration. diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 07142e9c24..2d9f360f16 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -8774,6 +8774,66 @@ def test_hook_condition_returns_false_without_raising(self, tmp_path): assert executor._evaluate_condition("config.x is set", "jira") is False +class TestConfigManagerTopLevelManifestDefaults: + """A top-level `defaults:` in extension.yml must be loaded (#5084). + + The extension template and EXTENSION-API-REFERENCE.md put defaults under a + top-level ``defaults:`` key, but ``_get_extension_defaults`` only read + ``config.defaults``, so an extension built from the template silently had + no defaults at runtime. + """ + + def _make(self, tmp_path, manifest_body: str): + ext_dir = tmp_path / ".specify" / "extensions" / "myext" + ext_dir.mkdir(parents=True) + (ext_dir / "extension.yml").write_text(manifest_body, encoding="utf-8") + return ConfigManager(tmp_path, "myext") + + def test_top_level_defaults_load(self, tmp_path): + cm = self._make(tmp_path, "defaults:\n feature:\n enabled: true\n") + assert cm.get_config() == {"feature": {"enabled": True}} + + def test_shipped_template_defaults_load(self, tmp_path): + """The defaults in extensions/template/extension.yml reach get_config().""" + template = ( + Path(__file__).resolve().parent.parent + / "extensions" / "template" / "extension.yml" + ).read_text(encoding="utf-8") + expected = yaml.safe_load(template)["defaults"] + assert expected, "template no longer ships example defaults" + cm = self._make(tmp_path, template) + assert cm.get_config() == expected + + def test_config_defaults_win_on_conflict(self, tmp_path): + cm = self._make( + tmp_path, + "defaults:\n feature:\n enabled: true\n auto_sync: true\n" + "config:\n defaults:\n feature:\n auto_sync: false\n", + ) + assert cm.get_config() == {"feature": {"enabled": True, "auto_sync": False}} + + def test_project_config_merges_over_top_level_defaults(self, tmp_path): + cm = self._make( + tmp_path, "defaults:\n feature:\n enabled: true\n auto_sync: true\n" + ) + (cm.extension_dir / "myext-config.yml").write_text( + "feature:\n enabled: false\n", encoding="utf-8" + ) + assert cm.get_config() == {"feature": {"enabled": False, "auto_sync": True}} + + def test_non_mapping_top_level_defaults_is_ignored(self, tmp_path): + cm = self._make( + tmp_path, + "defaults:\n - foo\nconfig:\n defaults:\n feature:\n enabled: true\n", + ) + assert cm.get_config() == {"feature": {"enabled": True}} + + def test_hook_condition_sees_top_level_default(self, tmp_path): + self._make(tmp_path, "defaults:\n feature:\n enabled: true\n") + executor = HookExecutor(tmp_path) + assert executor._evaluate_condition("config.feature.enabled is set", "myext") is True + + class TestConfigManagerEnvPrefixCollision: """Prefix-colliding env vars must not crash or clobber nested config.""" From ba091cc6171560eb46289d75241474f50e181a6f Mon Sep 17 00:00:00 2001 From: joshinitin2016883-sketch Date: Wed, 28 Oct 2026 22:44:50 +0530 Subject: [PATCH 2/2] test(extensions): keep top-level defaults when config is malformed Cover a valid top-level `defaults:` next to a non-mapping `config` or `config.defaults` (list and scalar), asserting the top-level defaults survive. Each pair fails if the matching fallback in _get_extension_defaults returns {} instead of the top-level defaults. Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous) Co-Authored-By: Claude Opus 5.5 --- tests/test_extensions.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 2d9f360f16..13350e2acb 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -8828,6 +8828,23 @@ def test_non_mapping_top_level_defaults_is_ignored(self, tmp_path): ) assert cm.get_config() == {"feature": {"enabled": True}} + @pytest.mark.parametrize( + "config_section", + [ + "config:\n - foo\n", + "config: just-a-string\n", + "config:\n defaults:\n - foo\n", + "config:\n defaults: just-a-string\n", + ], + ids=["list-config", "scalar-config", "list-defaults", "scalar-defaults"], + ) + def test_top_level_defaults_survive_non_mapping_config(self, tmp_path, config_section): + """A malformed `config` or `config.defaults` must not drop valid top-level defaults.""" + cm = self._make( + tmp_path, "defaults:\n feature:\n enabled: true\n" + config_section + ) + assert cm.get_config() == {"feature": {"enabled": True}} + def test_hook_condition_sees_top_level_default(self, tmp_path): self._make(tmp_path, "defaults:\n feature:\n enabled: true\n") executor = HookExecutor(tmp_path)