fix(workflows): report a falsy non-mapping overlay manifest as a shape error - #3884
Open
jawwad-ali wants to merge 1 commit into
Open
fix(workflows): report a falsy non-mapping overlay manifest as a shape error#3884jawwad-ali wants to merge 1 commit into
jawwad-ali wants to merge 1 commit into
Conversation
…e error
`ProjectOverlaySource.collect` did `yaml.safe_load(...) or {}`.
`validate_overlay_yaml` opens with an `isinstance(data, dict)` check, so a
truthy non-mapping is reported correctly — but `or {}` replaced the falsy
non-mappings with an empty mapping first, so those files were reported as
three bogus missing-field errors instead of the wrong shape:
'- a' -> ['Overlay manifest must be a mapping.']
'hello' -> ['Overlay manifest must be a mapping.']
'[]' -> ["Overlay 'id' is required...", "'extends' is required...",
"'edits' is required..."]
'false' -> same three
'0' -> same three
"''" -> same three
The sibling reader for these same files in the same package, `_read_overlay`
in overlays/_commands.py, does not coerce.
Only an empty document (None) now becomes an empty mapping, so a genuinely
empty overlay still reports its missing fields.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ProjectOverlaySource.collectinsrc/specify_cli/workflows/overlays/layer_sources.pyreads an overlay manifest with:validate_overlay_yamlopens withif not isinstance(data, dict): return None, ["Overlay manifest must be a mapping."]— so a wrong-shaped manifest is meant to be reported as exactly that. Butor {}replaces every falsy non-mapping with an empty mapping before that check runs, so those files are misreported.Reproduction on current
main(81bf741)Whole-document overlay manifests, same code path:
So
[],false,0and''produce three misleading "required field" errors, sending the author looking for missing keys in a document that has the wrong shape entirely. Their truthy twins get the right answer.Precedent
The sibling reader for these same files, in the same package —
_read_overlayinoverlays/_commands.py:160— does not coerce:So the two readers of one file format disagree, and the coercing one is the outlier.
Fix
Drop
or {}; normalise only an empty document:No breaking change.
None(an empty file) still becomes{}, so a genuinely empty overlay still reports its missing fields — pinned by a second new test. Every mapping is unaffected. The only inputs whose behaviour changes are the four falsy non-mappings, which move from three wrong errors to one right one.Verification
srcand pass with the fix — 8 failed → 4 failed, and those remaining 4 are Windows symlink-privilege tests that fail on cleanmain.tests/workflows: failure set identical to the clean-mainbaseline captured on81bf741(10 pre-existing in scope).uvx ruff@0.15.0 check src tests→ cleanNew class sits next to the existing
TestProjectOverlaySourceFileReadErrors, which covers the other failure modes of this same read.Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current
main.