feat: add parallel step type for concurrent workflow execution (#25)

[tbrandenburg]

Summary

Workflows previously executed steps strictly sequentially. This adds a declarative parallel step type that wraps child steps (vars, bash, agent) and runs them concurrently via a fork-join bash pattern (& + $! + wait). The generated artifact remains plain bash with no new runtime dependencies.

Root Cause

The Step union in models.py only included VarsStep | BashStep | AgentStep | ForStep, with no ParallelStep model or renderer branch.

Changes

File	Change
src/flowsh_cli/models.py	Add ParallelStep model with type: "parallel" and steps: list[Step] (min 1 child); update Step union; add model_rebuild()
src/flowsh_cli/render.py	Import ParallelStep; add elif isinstance(step, ParallelStep) branch with fork-join bash generation; add title branch in default_step_title()
tests/test_workflow_to_harness.py	Add 8 tests: YAML parse, empty-steps rejection, mixed child types, render structure assertions, section comments, harness execution, failure propagation, sequential coexistence

Testing

• Type check passes
• Unit tests pass (99/99)
• Lint passes
• Generated harness runs parallel steps concurrently
• Child failure propagates to wrapper exit code

Validation

make qa

Issue

Fixes #25

Implementation Details

Followed investigation artifact from issue #25 comment (tbrandenburg, 2026-06-03).
The ForStep precedent in the codebase was mirrored exactly: prefix-function emission for children, model_rebuild() after the Step union, and run_step (not run_stateful_step) for the parallel wrapper.

Deviations from plan

None — implementation followed the artifact exactly.

Automated implementation from investigation artifact

[tbrandenburg]

Automated Code Review

Summary

The implementation correctly adds ParallelStep support following the ForStep precedent exactly. All changes are minimal, focused, and do not touch existing functionality.

Findings

Strengths

• ParallelStep mirrors the ForStep pattern precisely: model_rebuild(), Field(min_length=1), prefix-function emission, and run_step (not run_stateful_step)
• Fork-join bash pattern ("fn" & → pid= → wait "$pid" || status=$?) is POSIX-portable and correct
• No changes to existing step types or their rendering — zero regression risk
• 8 focused tests covering parse, validation, render structure, execution, failure propagation, and sequential coexistence
• All 99 tests pass

Suggestions (non-blocking)

• render.py:260 — child_title is computed but only used in the section() comment; this is intentional and mirrors the ForStep inner-step title pattern
• Nested parallel inside parallel raises AssertionError from _render_step_body — acceptable per scope, but a ValidationError with a clear message would give a better user experience in a future iteration

Security

• No security concerns identified — no user input reaches shell unescaped; child function names go through step_function_name() which slugifies them

Checklist

• Fix addresses root cause from investigation (ParallelStep model + renderer branch)
• Code follows ForStep codebase pattern exactly
• Tests cover parse, render, execution, and failure propagation
• No obvious bugs introduced

Self-reviewed by Claude • Ready for human review

[tbrandenburg]

PR #27 Code Review — feat: add parallel step type for concurrent workflow execution

Author: tbrandenburg
Branch: fix/issue-25-parallel-step → main
Files changed: 4 (+314 / -3)
Recommendation: APPROVE

---

Summary

This PR introduces a parallel step type that wraps child steps (vars, bash, agent) and runs them concurrently using a POSIX-portable fork-join pattern (& + $! + wait). The generated artifact remains plain bash with no new runtime dependencies.

The implementation closely mirrors the ForStep precedent: model structure, model_rebuild() placement, Field(min_length=1) guard, prefix-function emission, and routing through run_step rather than run_stateful_step.

---

Validation Results

Check	Result	Detail
Lint (ruff check)	PASS	No warnings
Format (ruff format --check)	PASS	All files formatted
Syntax (py_compile)	PASS	All source files compile
Tests	PASS	99/99 passed in 20.3 s
Build	PASS	Wheel and sdist built cleanly

---

Strengths

• Minimal footprint: 10 lines to models.py and 31 lines to render.py. The approach is DRY — no duplication of runner boilerplate.
• Correct fork-join mechanics: background each child, capture $! into local pid_<name>, then wait "$pid" || status=$? for each. POSIX-portable and correct.
• Shared used_function_names set: parallel children are de-duplicated the same way as top-level steps, so name collisions (e.g., two children both named "Build") produce step_build + step_build_2 without stomping each other.
• Status propagation: last non-zero child exit code is returned from the wrapper, which is then surfaced by run_step via catch. Verified by test_generated_harness_parallel_step_propagates_child_failure.
• 8 focused tests: parse, empty-step rejection, mixed child types, render structure assertions, section comments, harness execution, failure propagation, and sequential coexistence. All exercising real bash execution — no mocks.
• AGENTS.md updated: the supported step-type list is kept in sync.
• No regressions: existing 91 tests continue to pass.

---

Issues Found

Medium — VarsStep inside parallel: exported variables silently disappear

File: src/flowsh_cli/render.py:254–272 / src/flowsh_cli/models.py:173

The model accepts VarsStep as a valid parallel child (it is listed in the Step union). However, parallel children run in subshells (via &). Any export VAR inside a subshell is invisible to the parent process. A user who writes:

- type: parallel
  steps:
    - type: vars
      values:
        RESULT: echo computed
    - type: bash
      run: echo $RESULT   # RESULT is never set here

will get a silently empty RESULT in subsequent steps, with no error.

Recommendation: Either (a) add a Pydantic validator on ParallelStep.steps that rejects VarsStep children with a clear error message, or (b) document the subshell-isolation semantics in the YAML model's field description so users are warned at authoring time. Option (a) is more robust.

---

Low — _render_step_body raises AssertionError for nested ParallelStep

File: src/flowsh_cli/render.py:345–348

_render_step_body handles ForStep with raise AssertionError("nested for steps are not supported"), but has no branch for ParallelStep. A nested parallel inside a parallel child falls through to the final else: raise AssertionError(f"Unsupported step type: {step}"). The message is correct but generic. The PR description acknowledges this is out of scope.

Recommendation: Add an explicit elif isinstance(step, ParallelStep): raise AssertionError("nested parallel steps are not supported") for parity with the ForStep branch. Same one-liner effort, better diagnostic message. Non-blocking.

---

Low — Parallel child naming uses top-level step_function_name instead of a namespaced variant

File: src/flowsh_cli/render.py:258

ForStep inner steps use _for_inner_function_name, which scopes children as for_{outer_index}_{slug}. This prevents a for-inner function from colliding with any top-level step name even before used_function_names deduplication.

Parallel children use step_function_name(i, child.name, used_function_names) directly, so a child named "Build" in a parallel block produces step_build — the same name a top-level step named "Build" would produce. The used_function_names set prevents actual collisions (the second registration gets _2), but the generated script may be slightly harder to read when names collide, since there's no parallel_{outer_index}_ prefix to visually scope them.

Recommendation: Consider a _parallel_child_function_name(outer_index, inner_index, step, used_function_names) helper that mirrors _for_inner_function_name. Non-blocking for this PR; worth doing in a follow-up for readability.

---

Low — Status propagation only captures last non-zero child exit code

File: src/flowsh_cli/render.py:271

wait "$pid_step_a" || status=$?
wait "$pid_step_b" || status=$?

If both children fail with different codes, only the second non-zero code is returned. This is the conventional POSIX behavior and is acceptable here, but it means catch only logs one failure code even when multiple children failed.

Recommendation: Document the behavior in a code comment (e.g., # last non-zero exit code wins; multiple failures logged individually by each child). Non-blocking.

---

Checklist

• PR addresses root cause (missing ParallelStep model + renderer branch)
• Implementation follows ForStep precedent exactly
• Validation passes: lint, format, syntax, 99 tests, build
• No security issues: child function names are slugified through step_function_name; no user input reaches shell unescaped
• No breaking changes to existing step types
• AGENTS.md updated to reflect new supported step type
• Tests cover parse, empty-step rejection, mixed types, render structure, execution, failure propagation, and coexistence with sequential steps

---

Decision

APPROVE — No critical or high issues. All validation passes. The implementation is clean, minimal, and follows established codebase patterns exactly. The three low/medium findings are documented above; the VarsStep semantic issue (Medium) is worth addressing in a follow-up before or after merge at the author's discretion.

---

Reviewed by OpenCode (claude-sonnet-4.6) — 2026-06-03