Skip to content

fix(workflows): reject falsy non-mapping step.yml in step add - #4321

Open
Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/step-add-falsy-yaml-shape
Open

fix(workflows): reject falsy non-mapping step.yml in step add#4321
Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/step-add-falsy-yaml-shape

Conversation

@Noor-ul-ain001

Copy link
Copy Markdown
Contributor

Summary

  • workflow_step_add (src/specify_cli/workflows/_commands.py) parses a fetched step.yml with _yaml.safe_load(step_yml_content.decode("utf-8")) or {}.
  • That or {} coerces a falsy non-mapping top-level document ([], false, 0, '') to {} before the if not isinstance(meta, dict): ... shape check runs. The command then proceeds with meta = {}, derives step_meta = meta.get("step", {}){} and type_key = "", and reports the unrelated "step.yml missing 'step.type_key' field" instead of the real problem: "step.yml must be a YAML mapping".
  • A truthy non-mapping document (e.g. a bare string) already hits the correct isinstance check and reports the right error — this was an inconsistency between falsy and truthy malformed inputs.
  • Same falsy-or-coerce shape as the catalog-config bugs already fixed this cycle in workflows/catalog.py (WorkflowCatalog/StepCatalog), presets/__init__.py (PresetCatalog._load_catalog_config), and integrations (fix(integrations): report a falsy non-mapping integration descriptor as a shape error #4187).

Test plan

  • Added test_add_rejects_falsy_non_mapping_step_yml to TestWorkflowStepAddCLI, parametrized over [], false, 0, '', mocking the HTTP fetch so step.yml's body is the falsy document.
  • Verified all 4 new parametrized cases fail without the fix (stashed only the source change, confirmed the misleading "missing 'step.type_key'" message) and pass with it.
  • Ran TestWorkflowStepAddCLI in full — 24 passed; the 1 remaining failure (test_add_rejects_symlinked_steps_base_dir) is a pre-existing Windows symlink-elevation failure unrelated to this change (reproduces on an unmodified checkout).

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01FW9fAYsCBCAgdKWovtSyqt

@Noor-ul-ain001
Noor-ul-ain001 requested a review from mnriem as a code owner August 25, 2026 15:07
`workflow_step_add` parses a fetched `step.yml` with
`_yaml.safe_load(...) or {}`, which coerces a FALSY non-mapping
top-level document (`[]`, `false`, `0`, `''`) to `{}` before the
`isinstance(meta, dict)` shape check runs. The command then proceeds
with `meta = {}`, derives `step_meta = {}` and `type_key = ""`, and
reports the unrelated "step.yml missing 'step.type_key' field"
instead of the real problem: "step.yml must be a YAML mapping". A
TRUTHY non-mapping document (a bare string) already reported the
correct error — this was an inconsistency.

Same falsy-or-coerce shape as the catalog-config bugs fixed elsewhere
in workflows/catalog.py, presets/__init__.py, and integrations
(github#4187) this cycle.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FW9fAYsCBCAgdKWovtSyqt
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