Skip to content

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

Merged
mnriem merged 1 commit into
github:mainfrom
jawwad-ali:fix/loop-require-steps
Sep 3, 2026
Merged

fix(workflows): require a steps body on while and do-while loops#4149
mnriem merged 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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The validation is consistent with sibling control-flow steps and is covered by focused regression tests.

Pull request overview

Requires explicit steps bodies for while and do-while workflow steps, preventing silent no-op loops.

Changes:

  • Adds missing-body validation to both loop types.
  • Adds regression tests for validation and prior runtime behavior.

Reviewed by GitHub Copilot.

File summaries
File Description
src/specify_cli/workflows/steps/while_loop/__init__.py Requires the steps field.
src/specify_cli/workflows/steps/do_while/__init__.py Requires the steps field.
tests/test_workflows.py Tests both missing-body cases.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mnriem
mnriem merged commit db64869 into github:main Sep 3, 2026
14 checks passed
@mnriem

mnriem commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

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.

3 participants