Skip to content

feat(workflows): compose workflows with a unified execution tree - #4764

Open
markuswondrak wants to merge 14 commits into
github:mainfrom
markuswondrak:feat/4680-composition-streamlined
Open

markuswondrak wants to merge 14 commits into
github:mainfrom
markuswondrak:feat/4680-composition-streamlined

Conversation

@markuswondrak

@markuswondrak markuswondrak commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Reimplements workflow composition from main (c00dc055) with one persisted execution tree and one executor. Replaces #4724 and implements the scoped single-run model approved in #4680 (comment), with the explicit resume extension described below.

Closes #4680.

Included workflows receive private, strictly bound inputs and return only declared outputs alongside workflow/status/error metadata. Targets must exactly match safe, installed, enabled IDs in the current project. Overlay-resolved definitions are bound at the call site, cycles are path-based, and included depth is limited to 16. The approved clarification supersedes the original issue's child-run and run_id-output proposal.

Current head: 6792bea1aaff941f1767a5c632da84d72f183337.

Why replace #4724

The previous implementation accumulated separate scope identities, result keys, cursor state, snapshot files, and execution paths. Its review identified a real fan-out isolation bug: nested calls had distinct scope keys but shared the caller-result alias.

Each execution occurrence now owns its result, children, and workflow binding. Authored step IDs are local expression aliases; concurrent items have independent contexts. Execution and replay traverse the same tree, including fan-out. Unused legacy execution adapters were removed; their tests exercise the public engine path.

The current diff from the merge base (c00dc055) is 17 files, 3,706 additions / 594 deletions, including 1,181 production additions / 486 deletions. These replace the smaller initial-PR figures: subsequent review added regression coverage and centralized traversal, projection, validation, and event rules. Local planning documents are not included.

Deliberate deviations

  • A1 — Exact, frozen resume. Resume continues at the unfinished occurrence. Chosen branches, loop iterations, fan-out items, custom expansions, and workflow bindings stay frozen; completed work is not re-run. This applies to all workflows because they share one executor. It goes beyond the approved single-run contract, which retained resume at the enclosing top-level step. Exact resume preserves completed child work and bound definitions across a pause, avoiding repeated command/LLM effects from restarting the enclosing step.
  • A2 — Fan-out item internals stay item-local. Internal item results no longer enter the shared root context. Public item results (fan:template:index) and the ordered fan.output.results remain available. This addresses the parallel-item collision in feat(workflows): compose installed workflows via a scoped workflow step #4724.

Architecture and contracts

  • _execution.py owns tree construction/validation, traversal/replay, control flow, workflow scope entry, projections, and event emission. composition.py owns target resolution, strict binding, declared outputs, and CallError. engine.py keeps public lifecycle, input coercion/merge, and RunState persistence.
  • YAML strings inside JSON preserve definition/expansion scalar types. Workflow children share the bound definition, fan-out items share the template, and later loop iterations share the first body positionally. There are no separate snapshot files or content-addressed identities.
  • Ordinary resume retains bound inputs and definitions, including after target changes/deactivation/removal. Explicit root input updates re-evaluate original mappings for reached, incomplete calls and merge them over prior bound inputs. Completed calls remain unchanged; unbound calls validate their targets when reached.
  • Parent inputs, step results, item/fan-in values, and workflow defaults are not implicitly inherited. Runtime conditions such as inside_fan_out are transitive. Child gates require explicit root-to-child verdict mapping.
  • continue_on_error can handle reported child failures and initial binding/output contract violations. Step exceptions, expression errors (including call input/output expressions), rebind errors, and checkpoint errors propagate. Pauses, aborts, and unknown step types remain terminal. A resolver RuntimeError is not converted into a call failure.
  • Output-finalization failures can retry without repeating completed child commands. Rebind failure leaves the call node and child subtree unchanged so corrected inputs can be retried.
  • Only PAUSED and FAILED runs can resume. The earlier crash-resume claim is withdrawn: RUNNING checkpoints are rejected. No ownership/lease mechanism is added. External effects before their completion checkpoint remain at-least-once.
  • Binding and chosen expansions are checkpointed before child effects; completion is checkpointed before logs. A checkpoint failure prevents further writes from that instance. Fan-out aliases are reconstructed projections, set under the existing run lock and saved by the next checkpoint.
  • Main-format legacy checkpoints adapt once from their top-level index. Private, unreleased formats from feat(workflows): compose installed workflows via a scoped workflow step #4724 are not migrated.
  • Nested gate/scope reporting follows the tree. Qualified IDs and private workflow/path attribution are shared by log emission; completed replay emits no step events. Unknown step implementations retain an internal retry record without publishing an item result.

Evidence

test_concurrent_nested_calls_keep_downstream_aliases_local was run against #4724 commit 7ece7a16 via an isolated import path. It failed with consumed outputs {1: 1, 2: 1} instead of {1: 1, 2: 2} and passes here.

Regression coverage includes call exception propagation, rebind preservation, transitive fan-out restrictions, frozen branch/custom expansion replay, qualified aliases/events, shared snapshots, malformed-tree rejection before writes, checkpoint/log failures, strict targets/inputs/outputs, depth/cycles, and legacy adaptation. The composed-gate CLI test covers run → JSON/human status → resume with explicitly mapped input.

The final fan-out fix extends test_unknown_fan_out_template_step_always_fails_despite_continue_on_error: four combinations (sequential/parallel, named/unnamed template) failed before the fix because missing implementations incorrectly published item aliases. All pass afterward, including a same-name inherited parent result and successful resume after re-registering the implementation. Direct comparison with c00dc055 now produces only the fan-out container result, with matching per-item failure events.

Current verification

  • uv sync --extra test, then this worktree's .venv/bin/python -m pytest tests/test_workflows.py tests/workflows tests/specify_cli/workflows -q -p no:cacheprovider: 1,373 passed, 1 skipped (26.25 seconds).
  • Focused fan-out/replay/concurrency selection: 18 passed.
  • uvx ruff@0.15.0 check src tests and git diff --check: pass.
  • Root unknown-step status/error/events and fan-out missing-step projections were directly compared with main c00dc055.

Historical evidence and remaining limitation

  • Clean-main workflow baseline: 1,259 passed, 1 skipped.
  • The full repository run on the initial rewrite was 8,435 passed, 212 skipped, 4 failed; all four also reproduced on clean c00dc055 (preset-update missing-argument wording and three locale-sensitive checksum expectations). The full repository suite has not been rerun after this cleanup. The current result above is for all workflow suites.
  • Recorded deterministic measurements against the initial PR show a 400-item fan-out dropping from 806 to 405 saves. With a 4,096-byte template, final state size dropped from 2,084,407 to 418,807 bytes. Sharing removes per-item duplication of definitions; progress still grows with item count. These are save/size measurements, not wall-clock guarantees.
  • The complete scenario-by-scenario main compatibility matrix remains incomplete. The passing suites and focused comparisons do not establish blanket equivalence beyond the specifically verified cases and documented A1/A2 changes.

Intentionally changed tests

  • test_checkpoint_failure_never_overwrites_committed_progress became test_checkpoint_failure_leaves_running_run_not_resumable; crash-resume expectations were removed/inverted when RUNNING resume was withdrawn.
  • test_rebind_failure_has_one_failed_caller_outcome now asserts propagation, run FAILED, and an unchanged call node rather than a recoverable call failure.
  • test_output_failure_retries_only_finalization injects CallError for a contract violation. test_output_expression_failure_retries_only_finalization separately covers propagating expression errors without repeating children.
  • Former private fan-out-adapter tests use public execute() while retaining their behavioral coverage.

Out of scope

Crash recovery/run ownership/leases, a dedicated expression-error type and consistent recoverability policy, a direct occurrence-addressed gate-answer API, implicit input propagation or parent-default inheritance, invalidation of completed dependent work, rejecting unknown root resume inputs, an execution-position value object, and migration of private PR checkpoint formats.

AI disclosure

Implemented and updated on behalf of @markuswondrak using OpenCode in autonomous mode with user-directed scope. This update used gpt-6-astra (github-copilot/gpt-6-astra) for review, the final fan-out fix and regression tests, automated verification, commit/push, and this fully AI-drafted PR description. Intermediate cleanup commits disclose gpt-5.6-terra and deepseek-v4.1-flash individually in their Assisted-by: trailers. The original rewrite and its AI-assisted #4724 history are retained. Human line-by-line review or manual testing is not attested.

Keep invocation results and workflow bindings on the same execution occurrence. Isolate fan-out contexts and resume persisted expansions through one executor.

Assisted-by: OpenCode (model: gpt-6-astra, autonomous)

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.

Copilot review overview

🟡 Changes recommended

Child step implementation exceptions are incorrectly converted into ordinary workflow failures and may be swallowed by continue_on_error.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Introduces composable workflows backed by a unified, persisted execution tree.

Changes:

  • Adds scoped workflow calls with typed inputs and declared outputs.
  • Unifies execution, resume, fan-out, and nested-state persistence.
  • Adds CLI reporting, documentation, and regression coverage.
File Description
src/​specify_cli/​workflows/​_commands.py Reports nested scopes and gates.
src/​specify_cli/​workflows/​_execution.py Implements tree-based execution.
src/​specify_cli/​workflows/​__init__.py Registers workflow steps.
src/​specify_cli/​workflows/​command_resume.py Documents crash recovery.
src/​specify_cli/​workflows/​command_status.py Displays composed scopes.
src/​specify_cli/​workflows/​composition.py Handles workflow boundaries.
src/​specify_cli/​workflows/​engine.py Integrates persistence and execution.
src/​specify_cli/​workflows/​step/​gate/​__init__.py Normalizes gate messages.
src/​specify_cli/​workflows/​step/​workflow/​__init__.py Defines the workflow step.
tests/​specify_cli/​workflows/​test_command_status.py Tests composed CLI lifecycle.
tests/​workflows/​test_composition_execution.py Covers composition and resume.
design/​workflow-step.md Updates execution guidance.
docs/​reference/​workflows.md Documents composition semantics.
workflows/​ARCHITECTURE.md Describes the execution tree.
workflows/​PUBLISHING.md Adds workflow-step validation guidance.
workflows/​README.md Lists the new step type.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/specify_cli/workflows/_execution.py Outdated
@mnriem mnriem added the triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate label Sep 27, 2026
Markus added 13 commits September 27, 2026 19:06
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Assisted-by: OpenCode (model: deepseek-v4.1-flash, autonomous)
Assisted-by: OpenCode (model: deepseek-v4.1-flash, autonomous)
Assisted-by: OpenCode (model: deepseek-v4.1-flash, autonomous)
Assisted-by: OpenCode (model: deepseek-v4.1-flash, autonomous)
Preserve the item traversal's projection decision for missing step types. Cover sequential and parallel execution, inherited aliases, unnamed templates, and resume after reinstalling the implementation.

Assisted-by: OpenCode (model: gpt-6-astra, autonomous)
Copilot AI review requested due to automatic review settings September 27, 2026 19:53
@markuswondrak

Copy link
Copy Markdown
Contributor Author

Updated through 6792bea1aaff941f1767a5c632da84d72f183337.

The cleanup narrows call-boundary recovery, preserves incomplete calls on rebind errors, removes RUNNING resume, unifies live/replay traversal and qualified events, validates inconsistent trees before writes, and shares immutable snapshots. The final fix prevents unknown fan-out step implementations from publishing item results while preserving resume after reinstallation.

Current workflow validation: 1,373 passed, 1 skipped; Ruff and git diff --check pass. The final regression's four sequential/parallel and named/unnamed cases failed before the fix and pass afterward. The PR description now explicitly documents exact frozen resume as an extension of the approved top-level-resume contract, the intentionally changed tests, and the remaining limitation: a complete scenario-by-scenario main comparison is not yet recorded. The full repository-suite numbers are identified as historical.

The error-handling follow-up proposal is explained in the reply to the review thread.

Posted on behalf of @markuswondrak by OpenCode (model: gpt-6-astra / github-copilot/gpt-6-astra, autonomous mode with user-directed scope); review summary and PR update fully AI-drafted, final fix and tests AI-authored and automatically verified. Earlier cleanup commits carry their own model disclosures.

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.

Copilot review overview

🟡 Changes recommended

Crash checkpoints remain marked running and are rejected by resume, contradicting the declared recovery contract.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment on lines +326 to +339
with self.state._lock:
if self.state._checkpoint_failed:
raise CheckpointError("A previous checkpoint failed")
if node is not None:
node.update(changes or {})
if context is not None and "result" in node:
self.project(
context,
name,
node["result"],
public=public,
qualified=qualified or name,
)
self.state.save()

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Compose workflows — run an installed workflow as a step

3 participants