Skip to content

Commit 3652663

Browse files
tbrandenburgOpenCode
andauthored
Fix: [Important] when: conditional field on steps for param-driven branching (#33) (#53)
* Fix: [Important] when: conditional field on steps for param-driven branching (#33) The workflow format had no step-level conditional guard, which forced separate workflows for entry-point variants like resume and from-scratch. Adding `when` lets a step skip itself based on a Bash expression while keeping the workflow running. Changes: - Added `when` to the shared step base model with validation - Rendered conditional skip guards in the generated harness - Added parser, render, and end-to-end tests for `when` Fixes #33 * Archive investigation for issue #33 * Fix: render when guards for nested steps (#33) --------- Co-authored-by: OpenCode <opencode@localhost>
1 parent 5541fe5 commit 3652663

4 files changed

Lines changed: 417 additions & 8 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Issue 33 Archive
2+
3+
## Issue
4+
5+
- #33: `[Important] when: conditional field on steps for param-driven branching`
6+
- Type: ENHANCEMENT
7+
8+
## Investigation Summary
9+
10+
The workflow model had no step-level conditional guard, so users had to duplicate whole workflows for entry-point variants like `--resume` and `--from-scratch`.
11+
12+
## Implementation Plan
13+
14+
- Add `when: str | None = None` to `BaseStep` in `src/flowsh_cli/models.py`
15+
- Validate `when` as a non-empty Bash expression string
16+
- Render a conditional skip guard in `src/flowsh_cli/render.py`
17+
- Add parser, render, and end-to-end tests in `tests/test_workflow_to_harness.py`
18+
19+
## Validation
20+
21+
- `make qa`
22+
23+
## Notes
24+
25+
The fix was implemented on branch `fix/issue-33-when-conditional-field`.

src/flowsh_cli/models.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class StrictModel(BaseModel):
4646

4747
class BaseStep(StrictModel):
4848
name: str | None = None
49+
when: str | None = None
4950

5051
@field_validator("name")
5152
@classmethod
@@ -56,6 +57,15 @@ def validate_optional_string(cls, value: str | None) -> str | None:
5657
raise ValueError("must not contain control characters")
5758
return value
5859

60+
@field_validator("when")
61+
@classmethod
62+
def validate_when(cls, value: str | None) -> str | None:
63+
if value is not None and value.strip() == "":
64+
raise ValueError("must not be empty")
65+
if value is not None and has_unsafe_control_characters(value):
66+
raise ValueError("must not contain unsafe control characters")
67+
return value
68+
5969

6070
class BashStep(BaseStep):
6171
type: Literal["bash"]

src/flowsh_cli/render.py

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ def render_step(index: int, step: Step, used_function_names: set[str] | None = N
241241
inner_title = inner_step.name or default_step_title(i, inner_step)
242242
prefix_lines.append(section(f"For-inner step ({inner_step.type}): {inner_title}"))
243243
prefix_lines.append(f"{inner_fn}() {{")
244-
prefix_lines.extend(_render_step_body(inner_step))
244+
prefix_lines.extend(_render_step_body(inner_step, inner_title))
245245
prefix_lines.append("}")
246246
prefix_lines.append("")
247247

@@ -259,7 +259,7 @@ def render_step(index: int, step: Step, used_function_names: set[str] | None = N
259259
child_fns.append(child_fn)
260260
prefix_lines.append(section(f"Parallel child {i} ({child.type}): {child_title}"))
261261
prefix_lines.append(f"{child_fn}() {{")
262-
prefix_lines.extend(_render_step_body(child))
262+
prefix_lines.extend(_render_step_body(child, child_title))
263263
prefix_lines.append("}")
264264
prefix_lines.append("")
265265

@@ -271,23 +271,36 @@ def render_step(index: int, step: Step, used_function_names: set[str] | None = N
271271
body_lines.append(f' wait "$pid_{child_fn}" || status=$?')
272272
body_lines.append(' return "$status"')
273273
else:
274-
body_lines = _render_step_body(step)
274+
body_lines = _render_step_body(step, title)
275275

276276
runner = "run_stateful_step" if isinstance(step, (VarsStep, ForStep)) else "run_step"
277277
outer_lines = [
278278
section(f"Step {index} ({step.type}): {title}"),
279279
f"{function_name}() {{",
280-
*body_lines,
281-
"}",
282-
f"{runner} {function_name}",
283-
"",
284280
]
281+
282+
outer_lines.extend(body_lines)
283+
outer_lines.extend(
284+
[
285+
"}",
286+
f"{runner} {function_name}",
287+
"",
288+
]
289+
)
285290
return prefix_lines + outer_lines
286291

287292

288-
def _render_step_body(step: Step) -> list[str]:
293+
def _render_step_body(step: Step, title: str) -> list[str]:
289294
"""Return indented body lines for a step function (no wrapper, no run_step call)."""
290295
lines: list[str] = []
296+
297+
if step.when is not None:
298+
lines.append(f" if ! ({step.when}); then")
299+
lines.append(f" log INFO {bash_quote(f'Step skipped (when): {title}')}")
300+
lines.append(" return 0")
301+
lines.append(" fi")
302+
lines.append("")
303+
291304
if isinstance(step, VarsStep):
292305
lines.append(" local status=0")
293306
for name, command in step.values.items():

0 commit comments

Comments
 (0)