Skip to content

Commit 74a561a

Browse files
Fix: [Critical] while loop step type for dynamic iteration (#31) (#54)
* Fix: [Critical] while loop step type for dynamic iteration (#31) The current for implementation snapshots its input before iteration, so workflows that create new planned items during the loop silently skip work and can produce incorrect results. Changes: - Added a first-class while step model with condition validation - Rendered while steps as stateful Bash loops that re-evaluate the condition each iteration - Documented while in the README - Added parser, render, and end-to-end tests Fixes #31 * Archive investigation for issue #31 --------- Co-authored-by: OpenCode <opencode@users.noreply.github.com>
1 parent c5a57ec commit 74a561a

5 files changed

Lines changed: 335 additions & 14 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Issue 31
2+
3+
**Title**: [Critical] while loop step type for dynamic iteration
4+
**Type**: BUG
5+
**Investigated**: 2026-06-27T19:16:56Z
6+
7+
## Summary
8+
9+
`for` loops snapshot their input before iteration, so workflows that create new work during the loop do not pick it up in the same run.
10+
11+
## Implementation Plan
12+
13+
- Add a `while` step model with `condition` and nested `steps`.
14+
- Render `while` as a Bash loop that re-evaluates the condition each iteration.
15+
- Add parser, render, and end-to-end tests.
16+
- Document `while` in the README.
17+
18+
## Validation
19+
20+
- `make qa`
21+
22+
## Notes
23+
24+
This archive reflects the investigation comment used as the implementation plan.

README.md

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,18 +52,23 @@ workflows:
5252
expandPrompt: true
5353
prompt: |
5454
Review issue ${ISSUE_NUMBER} and summarize the repository state.
55-
- type: parallel
56-
steps:
57-
- type: bash
58-
run: echo "worker A"
59-
- type: bash
60-
run: echo "worker B"
61-
- type: for
62-
in: ITEMS
63-
item: ITEM
64-
steps:
65-
- type: bash
66-
run: echo "$ITEM"
55+
- type: parallel
56+
steps:
57+
- type: bash
58+
run: echo "worker A"
59+
- type: bash
60+
run: echo "worker B"
61+
- type: for
62+
in: ITEMS
63+
item: ITEM
64+
steps:
65+
- type: bash
66+
run: echo "$ITEM"
67+
- type: while
68+
condition: '[ -n "$(ls doc/plan/steps/planned/ 2>/dev/null)" ]'
69+
steps:
70+
- type: bash
71+
run: echo "keep looping until the queue is empty"
6772
```
6873
6974
Harness paths are derived from workflow ids. `wf_example` becomes `example.sh` in the current working directory.
@@ -76,6 +81,7 @@ Harness paths are derived from workflow ids. `wf_example` becomes `example.sh` i
7681
| `bash` | Run shell commands | Runs with `bash -euo pipefail`. |
7782
| `agent` | Call OpenCode | Supports `agent`, `model`, `command`, `expandPrompt`, and `dangerouslySkipPermissions`. |
7883
| `for` | Iterate over newline-delimited values from a previous `vars` step | Flat iteration only; nested `for` steps are not supported. |
84+
| `while` | Re-evaluate a Bash condition before each iteration | Use for dynamic queues or other stateful loops that must keep discovering new work. |
7985
| `parallel` | Run child steps concurrently | Children run as separate branches and the parent waits for all of them. |
8086

8187
## Agent Behavior

src/flowsh_cli/models.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,18 +196,42 @@ def validate_var_name(cls, value: str) -> str:
196196
return value
197197

198198

199+
class WhileStep(BaseStep):
200+
type: Literal["while"]
201+
condition: str
202+
steps: list[Step] = Field(min_length=1)
203+
204+
@field_validator("condition")
205+
@classmethod
206+
def validate_condition(cls, value: str) -> str:
207+
if value.strip() == "":
208+
raise ValueError("must not be empty")
209+
if has_unsafe_control_characters(value):
210+
raise ValueError("must not contain unsafe control characters")
211+
return value
212+
213+
@field_validator("steps")
214+
@classmethod
215+
def reject_nested_while_steps(cls, value: list[Step]) -> list[Step]:
216+
for step in value:
217+
if isinstance(step, WhileStep):
218+
raise ValueError("nested while steps are not supported")
219+
return value
220+
221+
199222
class ParallelStep(BaseStep):
200223
type: Literal["parallel"]
201224
steps: list[Step] = Field(min_length=1)
202225

203226

204227
Step = Annotated[
205-
VarsStep | BashStep | AgentStep | ForStep | ParallelStep,
228+
VarsStep | BashStep | AgentStep | ForStep | WhileStep | ParallelStep,
206229
Field(discriminator="type"),
207230
]
208231

209232

210233
ForStep.model_rebuild()
234+
WhileStep.model_rebuild()
211235
ParallelStep.model_rebuild()
212236

213237

src/flowsh_cli/render.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
ParallelStep,
1111
Step,
1212
VarsStep,
13+
WhileStep,
1314
Workflow,
1415
WorkflowParam,
1516
)
@@ -251,6 +252,30 @@ def render_step(index: int, step: Step, used_function_names: set[str] | None = N
251252
*[f" run_step {fn}" for fn in inner_fns],
252253
f' done <<< "${{{step.in_}}}"',
253254
]
255+
elif isinstance(step, WhileStep):
256+
child_fns: list[str] = []
257+
for i, child in enumerate(step.steps, start=1):
258+
child_title = child.name or default_step_title(i, child)
259+
child_fn = step_function_name(i, child.name, used_function_names)
260+
child_fns.append(child_fn)
261+
prefix_lines.append(section(f"While child {i} ({child.type}): {child_title}"))
262+
prefix_lines.append(f"{child_fn}() {{")
263+
prefix_lines.extend(_render_step_body(child, child_title))
264+
prefix_lines.append("}")
265+
prefix_lines.append("")
266+
267+
body_lines = []
268+
if step.when is not None:
269+
body_lines.append(f" if ! ({step.when}); then")
270+
body_lines.append(f" log INFO {bash_quote(f'Step skipped (when): {title}')}")
271+
body_lines.append(" return 0")
272+
body_lines.append(" fi")
273+
body_lines.append("")
274+
275+
body_lines.append(f" while ({step.condition}); do")
276+
for child_fn in child_fns:
277+
body_lines.append(f" run_step {child_fn} || return $?")
278+
body_lines.append(" done")
254279
elif isinstance(step, ParallelStep):
255280
child_fns: list[str] = []
256281
for i, child in enumerate(step.steps, start=1):
@@ -273,7 +298,7 @@ def render_step(index: int, step: Step, used_function_names: set[str] | None = N
273298
else:
274299
body_lines = _render_step_body(step, title)
275300

276-
runner = "run_stateful_step" if isinstance(step, (VarsStep, ForStep)) else "run_step"
301+
runner = "run_stateful_step" if isinstance(step, (VarsStep, ForStep, WhileStep)) else "run_step"
277302
outer_lines = [
278303
section(f"Step {index} ({step.type}): {title}"),
279304
f"{function_name}() {{",
@@ -393,6 +418,8 @@ def default_step_title(index: int, step: Step) -> str:
393418
return truncate_one_line(step.prompt)
394419
if isinstance(step, ForStep):
395420
return f"for {step.item} in {step.in_}"
421+
if isinstance(step, WhileStep):
422+
return f"while {truncate_one_line(step.condition)}"
396423
if isinstance(step, ParallelStep):
397424
return f"parallel ({len(step.steps)} steps)"
398425
return f"step {index}"

0 commit comments

Comments
 (0)