From 895dcf51fda11677898c446984588e909f7b9707 Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Tue, 18 Aug 2026 21:23:29 +0500 Subject: [PATCH 1/4] fix(integrations): report a falsy non-mapping integration descriptor as a shape error `IntegrationDescriptor._load` did `yaml.safe_load(fh) or {}`. `_validate` opens with an `isinstance(self.data, dict)` check, so a truthy non-mapping (`- a`, `hello`) is reported correctly -- but `or {}` replaced the falsy non-mappings with an empty mapping first, so those descriptors were reported as "Missing required field: schema_version" instead of the wrong shape: 'false' -> Descriptor root must be a YAML mapping, got bool '0' -> Descriptor root must be a YAML mapping, got int "''" -> Descriptor root must be a YAML mapping, got str '[]' -> Descriptor root must be a YAML mapping, got list `safe_load` also returns None for an explicit null scalar (`null`, `~`, `NULL`) as well as for an empty document, so those three hit the same masking. Use `yaml.compose`, which yields no node only for a genuinely empty document, to tell the two apart -- only an empty document still normalizes to `{}` and reports its missing fields. Same bug class just fixed in the sibling overlay-manifest loader (upstream commit 39c36c4, PR #3884); this is the unfixed twin in the integration catalog's descriptor loader. Co-Authored-By: Claude Sonnet 5 --- src/specify_cli/integrations/catalog.py | 23 +++++++++++--- .../integrations/test_integration_catalog.py | 30 +++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/integrations/catalog.py b/src/specify_cli/integrations/catalog.py index e18d30a6fa..997dc0b488 100644 --- a/src/specify_cli/integrations/catalog.py +++ b/src/specify_cli/integrations/catalog.py @@ -674,16 +674,31 @@ def __init__(self, descriptor_path: Path) -> None: @staticmethod def _load(path: Path) -> dict: try: - with open(path, "r", encoding="utf-8") as fh: - return yaml.safe_load(fh) or {} - except yaml.YAMLError as exc: - raise IntegrationDescriptorError(f"Invalid YAML in {path}: {exc}") + text = path.read_text(encoding="utf-8") except FileNotFoundError: raise IntegrationDescriptorError(f"Descriptor not found: {path}") except (OSError, UnicodeError) as exc: raise IntegrationDescriptorError( f"Unable to read descriptor {path}: {exc}" ) + try: + # ``safe_load`` returns None for BOTH an empty document and an + # explicit null scalar (``null``, ``~``, ``Null``, ``NULL``), so it + # cannot tell them apart on its own. ``compose`` yields no node + # only for a genuinely empty document. + is_empty_document = yaml.compose(text) is None + data = yaml.safe_load(text) + except yaml.YAMLError as exc: + raise IntegrationDescriptorError(f"Invalid YAML in {path}: {exc}") + # Only a genuinely EMPTY document becomes an empty mapping, so its + # missing-field errors are reported. Every non-mapping document -- + # including an explicit ``null``/``~`` and the falsy shapes ``[]``, + # ``false``, ``0``, ``''`` that a plain ``or {}`` would mask -- must + # reach ``_validate`` unchanged so it reports the wrong descriptor + # shape, like the truthy twins (``- a``, ``hello``) already do. + if is_empty_document: + data = {} + return data # -- Validation ------------------------------------------------------- diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index 9b02632992..a17eeafae6 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -700,6 +700,36 @@ def test_scripts_not_a_list(self, tmp_path): with pytest.raises(IntegrationDescriptorError, match="expected a list"): IntegrationDescriptor(p) + @pytest.mark.parametrize( + "content", ["[]", "false", "0", "''", "null", "~", "NULL", "- a", "hello"] + ) + def test_falsy_non_mapping_descriptor_reports_shape_error(self, tmp_path, content): + """Every non-mapping document reports the mapping-shape error. + + `_validate` opens with an `isinstance(self.data, dict)` check, so a + truthy non-mapping (`- a`, `hello`) correctly reported "Descriptor root + must be a YAML mapping". `_load`'s plain `yaml.safe_load(fh) or {}` + masked that for the falsy shapes `[]`, `false`, `0`, `''` (coerced to + an empty mapping) and for an explicit null scalar (`null`, `~`, `NULL` + -- indistinguishable from an empty document by `safe_load` alone), so + those five reported "Missing required field: schema_version" instead. + """ + p = tmp_path / "integration.yml" + p.write_text(content) + with pytest.raises( + IntegrationDescriptorError, + match="Descriptor root must be a YAML mapping", + ): + IntegrationDescriptor(p) + + def test_empty_document_still_reports_missing_fields(self, tmp_path): + """An empty document is not a wrong shape -- it is a mapping with no + keys, so the missing-field error must still be what is reported.""" + p = tmp_path / "integration.yml" + p.write_text("") + with pytest.raises(IntegrationDescriptorError, match="Missing required field: schema_version"): + IntegrationDescriptor(p) + def test_file_not_found(self, tmp_path): with pytest.raises(IntegrationDescriptorError, match="Descriptor not found"): IntegrationDescriptor(tmp_path / "nonexistent.yml") From 1b72be698abff773b11b18b86a4d41e013f485f3 Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Wed, 19 Aug 2026 17:24:07 +0500 Subject: [PATCH 2/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/specify_cli/integrations/catalog.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/integrations/catalog.py b/src/specify_cli/integrations/catalog.py index 997dc0b488..a7010a56b1 100644 --- a/src/specify_cli/integrations/catalog.py +++ b/src/specify_cli/integrations/catalog.py @@ -686,8 +686,14 @@ def _load(path: Path) -> dict: # explicit null scalar (``null``, ``~``, ``Null``, ``NULL``), so it # cannot tell them apart on its own. ``compose`` yields no node # only for a genuinely empty document. - is_empty_document = yaml.compose(text) is None - data = yaml.safe_load(text) +node = yaml.compose(text) +data = yaml.safe_load(text) +is_empty_document = node is None or ( + data is None + and isinstance(node, yaml.nodes.ScalarNode) + and node.value == "" + and node.start_mark.index == node.end_mark.index +) except yaml.YAMLError as exc: raise IntegrationDescriptorError(f"Invalid YAML in {path}: {exc}") # Only a genuinely EMPTY document becomes an empty mapping, so its From f59ff03142b142e536319fb76834662c59720fe7 Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Wed, 19 Aug 2026 19:55:53 +0500 Subject: [PATCH 3/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/specify_cli/integrations/catalog.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/integrations/catalog.py b/src/specify_cli/integrations/catalog.py index a7010a56b1..e93dab5185 100644 --- a/src/specify_cli/integrations/catalog.py +++ b/src/specify_cli/integrations/catalog.py @@ -686,14 +686,14 @@ def _load(path: Path) -> dict: # explicit null scalar (``null``, ``~``, ``Null``, ``NULL``), so it # cannot tell them apart on its own. ``compose`` yields no node # only for a genuinely empty document. -node = yaml.compose(text) -data = yaml.safe_load(text) -is_empty_document = node is None or ( - data is None - and isinstance(node, yaml.nodes.ScalarNode) - and node.value == "" - and node.start_mark.index == node.end_mark.index -) + node = yaml.compose(text) + data = yaml.safe_load(text) + is_empty_document = node is None or ( + data is None + and isinstance(node, yaml.nodes.ScalarNode) + and node.value == "" + and node.start_mark.index == node.end_mark.index + ) except yaml.YAMLError as exc: raise IntegrationDescriptorError(f"Invalid YAML in {path}: {exc}") # Only a genuinely EMPTY document becomes an empty mapping, so its From fba05759be80a0ab4b0526552ba526c961c0d3a6 Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Wed, 19 Aug 2026 20:31:17 +0500 Subject: [PATCH 4/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tests/integrations/test_integration_catalog.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integrations/test_integration_catalog.py b/tests/integrations/test_integration_catalog.py index a17eeafae6..87ab98a4d0 100644 --- a/tests/integrations/test_integration_catalog.py +++ b/tests/integrations/test_integration_catalog.py @@ -722,11 +722,11 @@ def test_falsy_non_mapping_descriptor_reports_shape_error(self, tmp_path, conten ): IntegrationDescriptor(p) - def test_empty_document_still_reports_missing_fields(self, tmp_path): - """An empty document is not a wrong shape -- it is a mapping with no - keys, so the missing-field error must still be what is reported.""" + @pytest.mark.parametrize("content", ["", "---"]) + def test_empty_document_still_reports_missing_fields(self, tmp_path, content): + """Empty documents are normalized to an empty mapping, so missing fields are reported.""" p = tmp_path / "integration.yml" - p.write_text("") + p.write_text(content) with pytest.raises(IntegrationDescriptorError, match="Missing required field: schema_version"): IntegrationDescriptor(p)