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..13350e2acb 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -8774,6 +8774,83 @@ 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}} + + @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) + 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."""