mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(bundler): treat an explicit-null manifest field as missing, not the text "None" (#3798)
BundleManifest.from_dict read every required scalar as
`str(raw.get(key, "")).strip()`. The `""` default only covers a MISSING key. A
key present but null -- exactly how YAML spells an empty field (`author:` with
nothing after it) -- yields None, and `str(None)` is the literal string "None".
That value is non-empty, so it sailed past the `if not value` required-field
checks in structural_errors().
Reproduced on main:
bundle.yml with description:/author:/license: left empty
-> description='None' author='None' license='None'
-> structural_errors() == []
-> specify bundle validate: exit 0, "demo is well-formed and valid."
So an empty required field was silently accepted and the bundle shipped the
literal text "None" as its author/license/description -- which is what
`bundle info` and a catalog entry then display. A null `provides.<kind>[].id`
likewise became a component literally named "None".
Add a `_text()` helper beside the existing `_parse_str_list` (the file's
established "one coercion helper applied at every site" shape) mapping an
explicit null to "", and route the required scalars through it. Same
silent-acceptance class as the already-merged guards in this function: #3629
(non-mapping `integration:`) and #3661 (falsy non-mapping requires/provides).
Non-null values are still `str()`-coerced and stripped, and an absent key
already produced "" -- so valid manifests are byte-for-byte unaffected.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -26,6 +26,45 @@ def test_missing_required_field_is_reported_by_name():
|
||||
assert any("bundle.license" in e for e in errors)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"field", ["name", "role", "description", "author", "license"]
|
||||
)
|
||||
def test_explicit_null_bundle_field_is_reported_as_missing(field):
|
||||
"""A field present but null is how YAML spells an empty value (`author:`).
|
||||
|
||||
`str(None)` is the literal text "None", which is non-empty, so it passed the
|
||||
required-field checks: the bundle validated clean and shipped "None" as its
|
||||
author/license/description.
|
||||
"""
|
||||
data = valid_manifest_dict()
|
||||
data["bundle"][field] = None
|
||||
manifest = BundleManifest.from_dict(data)
|
||||
assert getattr(manifest.bundle, field) == ""
|
||||
assert any(f"bundle.{field}" in e for e in manifest.structural_errors())
|
||||
|
||||
|
||||
def test_explicit_null_speckit_version_is_reported_as_missing():
|
||||
data = valid_manifest_dict()
|
||||
data["requires"]["speckit_version"] = None
|
||||
manifest = BundleManifest.from_dict(data)
|
||||
assert manifest.requires.speckit_version == ""
|
||||
assert any("speckit_version" in e for e in manifest.structural_errors())
|
||||
|
||||
|
||||
def test_explicit_null_component_id_is_not_named_none():
|
||||
"""A null component id must not become a component literally named "None"."""
|
||||
data = valid_manifest_dict()
|
||||
for kind, items in (data.get("provides") or {}).items():
|
||||
if isinstance(items, list) and items and isinstance(items[0], dict):
|
||||
items[0]["id"] = None
|
||||
break
|
||||
else: # pragma: no cover - fixture is expected to provide components
|
||||
pytest.skip("fixture has no component list to null out")
|
||||
manifest = BundleManifest.from_dict(data)
|
||||
assert manifest.components, "fixture is expected to declare components"
|
||||
assert all(ref.id != "None" for ref in manifest.components)
|
||||
|
||||
|
||||
def test_unsupported_schema_version_is_rejected():
|
||||
data = valid_manifest_dict(schema_version="9.9")
|
||||
errors = BundleManifest.from_dict(data).structural_errors()
|
||||
|
||||
Reference in New Issue
Block a user