Skip to content

fix(bundler): treat an explicit-null manifest field as missing, not the text "None" - #3798

Open
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/manifest-explicit-null-fields
Open

fix(bundler): treat an explicit-null manifest field as missing, not the text "None"#3798
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/manifest-explicit-null-fields

Conversation

@jawwad-ali

Copy link
Copy Markdown
Contributor

Problem

BundleManifest.from_dict reads every required scalar as str(raw.get(key, "")).strip(). The "" default only covers a missing key. A key that is present but null — exactly how YAML spells an empty field — yields None, and str(None) is the literal string "None". That value is non-empty, so it sails straight past the if not value required-field checks in structural_errors().

bundle:
  id: demo
  name: Demo
  version: 1.0.0
  role: developer
  description:      # <- empty
  author:           # <- empty
  license:          # <- empty

Reproduced on main:

description='None'   author='None'   license='None'
structural_errors() == []
specify bundle validate -> exit 0: "demo is well-formed and valid."

So an empty required field is silently accepted, and the bundle ships the literal text "None" as its author/license/description — which is what bundle info and a catalog entry then display to users. A null provides.<kind>[].id likewise became a component literally named "None".

Fix

Add a _text() helper beside the existing _parse_str_list — matching this file's established "one coercion helper, applied at every site" shape — that maps an explicit null to "", and route the required scalars (schema_version, the six bundle.* fields, requires.speckit_version, component id) through it.

After:

description=''  author=''
structural_errors():
  - Missing required field: bundle.description.
  - Missing required field: bundle.author.
  - Missing required field: bundle.license.

This is the same silent-acceptance class as the already-merged guards in this exact function — #3629 (present-but-non-mapping integration: was silently dropped) and #3661 (falsy non-mapping requires/provides).

Not a behaviour change for valid input

_text is equivalent to the code it replaces for every input except an explicit None: an absent key already produced "", and non-null scalars are still str()-coerced and stripped. Verified — a valid manifest still yields structural_errors() == [] with author='Ali'.

Verification

  • test_explicit_null_bundle_field_is_reported_as_missing (5 parametrized fields) + null speckit_version + null component id: 7 fail before, pass after.
  • tests/contract/test_manifest_schema.py: 35 passed (28 pre-existing untouched).
  • tests/contract/ + tests/unit/: 272 passed — the 3 remaining failures are pre-existing Windows symlink-privilege tests (need admin; pass on CI's Linux/macOS runners).
  • uvx ruff@0.15.0 check src tests clean.

AI-assisted: authored with Claude Code. I reproduced the "None" values and the clean bundle validate on main first, then confirmed valid manifests are unaffected after the change.

@jawwad-ali
jawwad-ali requested a review from mnriem as a code owner July 28, 2026 14:27
…he text "None"

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: github#3629
(non-mapping `integration:`) and github#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>
@jawwad-ali
jawwad-ali force-pushed the fix/manifest-explicit-null-fields branch from 0988775 to ba47a6a Compare July 28, 2026 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant