Skip to content

fix(workflows): require a steps body on while and do-while loops - #4149

Open
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/loop-require-steps
Open

fix(workflows): require a steps body on while and do-while loops#4149
jawwad-ali wants to merge 1 commit into
github:mainfrom
jawwad-ali:fix/loop-require-steps

Conversation

@jawwad-ali

Copy link
Copy Markdown
Contributor

Problem

Both loop steps type-check steps ('steps' must be a list) but never require it to be present, so an absent body silently becomes [].

Every sibling control-flow step already requires its body:

step required body key enforced
if then if_then/__init__.py:84
fan-out items + step fan_out/__init__.py:87, 92
while steps
do-while steps

The mistype is unusually easy here: the fan-out step's own payload key is the singular step: while the loops use steps:.

Reproduction on current main

A. while, body key typo'd as singular step:
   validate: []
   execute : StepStatus.COMPLETED | next_steps = []
B. do-while, no steps at all:
   validate: []
   execute : StepStatus.COMPLETED | next_steps = []

specify workflow validate reports zero errors. At run time the step reports COMPLETED while returning no next_steps, so the engine's if result.next_steps: block never fires and the retry loop the workflow is built around never runs even once — a silent no-op with no diagnostic anywhere.

For do-while this also contradicts the step's own docstring: "The first invocation always returns the nested steps for execution."

Fix

Add the missing presence check to both validate() methods, worded to match the existing if step's message.

Behaviour change — disclosed

A workflow that today declares a while/do-while with no steps key now fails specify workflow validate instead of passing. That is the point of the fix: such a loop is already a guaranteed no-op, so the change converts a silent misconfiguration into a clear error rather than altering any working behaviour. Runtime execution is untouched.

I scanned every workflow-shaped YAML in the repo — 0 bodyless loops, so no in-tree workflow or template is affected. An explicitly empty steps: [] still validates exactly as before.

Verification

  • Fail-before / pass-after: with both source files reverted to upstream/main, the 2 new tests fail → 37 passed with the fix.
  • Scoped regression across the loop/validate/engine surface (tests/test_workflows.py + tests/workflows, -k "While or DoWhile or Loop or loop or Validat or validate or Engine"): 283 passed.
  • Both existing test_validate_missing_fields tests already pass "steps": [], so they are unaffected.
  • Each new test also asserts the consequence — that execution is a silent no-op — not just the message.
  • uvx ruff@0.15.0 check src tests → clean

Written with assistance from Claude Code. Bug found, reproduced, and verified by me on current main.

Both loop steps type-check `steps` ("must be a list") but never require it
to be present, so an absent body silently becomes `[]`. `if` already
requires `then`, and `fan-out` already requires both `items` and `step`.

The mistype is unusually easy here because the fan-out step's own payload
key is the singular `step:` while the loops use `steps:`. Writing `step:` on
a `while` passed `specify workflow validate` with zero errors:

  A. while, body key typo'd as singular step:
     validate: []
     execute : StepStatus.COMPLETED | next_steps = []
  B. do-while, no steps at all:
     validate: []
     execute : StepStatus.COMPLETED | next_steps = []

At run time the step reports COMPLETED while returning no `next_steps`, so
the engine's `if result.next_steps:` block never fires and the loop the
workflow is built around never runs even once. `DoWhileStep`'s own docstring
promises "The first invocation always returns the nested steps for
execution".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jawwad-ali
jawwad-ali requested a review from mnriem as a code owner August 15, 2026 16:31
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