fix(workflows): if-step validate accepts falsy non-list else (#3264)

* fix(workflows): if-step validate accepts falsy non-list else

IfThenStep.validate() guarded the 'else' branch with
'if else_branch and not isinstance(else_branch, list)'. The leading
truthiness check short-circuits for falsy non-list values (False, 0,
'', {}), so a malformed else-branch passes validation and is then
silently skipped at runtime. The sibling 'then' branch is validated
strictly; 'else' now matches by switching to an 'is not None' guard.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(workflows): cover explicit else:None and missing-else separately

Per Copilot feedback: the parametrized valid-else test omitted the
'else' key when the value was None, so it covered only the missing-else
case, not an explicit 'else: None'. Set 'else' explicitly (including
None) in the parametrized test and add a dedicated missing-else test, so
both accepted shapes are pinned.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Ali jawwad
2026-07-13 20:14:55 +05:00
committed by GitHub
parent 7ff4522cf3
commit 55c66125f0
2 changed files with 42 additions and 2 deletions

View File

@@ -47,8 +47,8 @@ class IfThenStep(StepBase):
errors.append(
f"If step {config.get('id', '?')!r}: 'then' must be a list of steps."
)
else_branch = config.get("else", [])
if else_branch and not isinstance(else_branch, list):
else_branch = config.get("else")
if else_branch is not None and not isinstance(else_branch, list):
errors.append(
f"If step {config.get('id', '?')!r}: 'else' must be a list of steps."
)