You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Reimplements workflow composition from main (originally c00dc055; upstream/main has since been merged, so the merge base is now 2c0a57ab) 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.
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.
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 (2c0a57ab) is 20 files, 4,860 additions / 600 deletions, including 1,269 production additions / 492 deletions under src/. 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.
Every occurrence, including a workflow call, is checkpointed as the active step (current_step_id, qualified as in events) before its start is logged and before it executes. Both step paths share one Execution.start() sequence, as main does in _execute_steps.
Binding and chosen expansions are checkpointed before child effects; completion is checkpointed before logs. A checkpoint write failure prevents further writes from that instance. A graceful interrupt (KeyboardInterrupt), including one raised during a checkpoint, is not a checkpoint failure: it reaches the pause path as on main, which re-saves the tree from memory. Fan-out aliases are reconstructed projections, set under the existing run lock and saved by the next checkpoint.
Fan-out replay reproduces the live item view: a resumed item sees the same context as in a run that never paused (the engine-added results of a halted fan-out stay in the reporting view only), and an already-aborted sibling replays its stored result instead of publishing {}.
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.
Fan-out item aliases (fan:template:index) are reporting-only, not fan-in.wait_for targets. wait_for again requires declared step IDs (the fan-out step's own id; ordered item results at steps.<id>.output.results), matching main. The fan_out_aliases validation allowance is reverted, and FanInStep.execute() rejects : entries at runtime. This closes a stale-read path: in a loop iteration ≥ 1 the authored item alias was never refreshed, so a per-item join silently read the previous iteration's values.
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 reporting-only change is covered by test_fan_in_rejects_fan_out_item_alias (validation) and test_fan_in_rejects_item_alias_at_runtime (unvalidated execute), plus test_composed_fan_out_joins_container_results and test_fan_in_container_join_in_loop_sees_current_iteration as container-join controls. A loop with differing per-iteration values previously let a per-item join read iteration 0; the construct is now unexpressible.
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.
Review rounds since 9375b86e
For 3e5e66e8, e5a4223b, 8e9c5fa4, and 4db5a079, the new tests were shown to fail before the change. "Compared with main" means the same script was run against 67ab049e, whose workflow execution code matches the merge base 2c0a57ab apart from workflow-version validation.
3c503260: no log writes after a checkpoint failure; an unknown child step is a reported call failure.
f2dca00f: resume snapshot check covered with native YAML scalars (.nan); no production change.
97874f3c, 2c0e1415: current_step_id is recomputed on exception paths and uses qualified occurrence IDs; gate messages keep typed values; a fan-out worker exception stops further dispatch.
1b0a3a46, 3e5e66e8: the active occurrence is checkpointed before it starts; 3e5e66e8 extends this to workflow calls, which 1b0a3a46 missed because calls take a separate path.
e5a4223b: fixes a regression from 7aa424dd where RunState.save() caught BaseException, so a KeyboardInterrupt during a checkpoint left the run running and not resumable. main pauses.
8e9c5fa4: fixes a regression from 6792bea1 where a parallel fan-out's already-aborted item lost its output ({}) when a sibling resumed.
4db5a079: resumed fan-out items no longer see the halted fan-out's partial results (present since 7aa424dd; main never exposes them). test_resumed_fan_out_item_sees_its_uninterrupted_context checks the general invariant for step, if-container, and workflow-call templates, sequentially and in parallel; results was the only difference it found.
Reported in review, reproduced unchanged on main, and deferred (tracked in markuswondrak#4): a concurrent fan-out worker exception can be dropped after an earlier item halts; steps nested in if/switch inside a fan-out or loop lose the occurrence qualifier; gate messages that are not lossless JSON (for example {1: one}) change after reload.
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,459 passed, 1 skipped.
uvx ruff@0.15.0 check src tests and git diff --check: pass.
CI on e5a4223b: ruff, Lint, CodeQL, Security Audit, and Extension Version Guard passed. One pytest (macos-latest, 3.14) job failed on tests/specify_cli/workflows/test_catalog_versions.py::test_exact_add_uses_historical_url_digest_and_requirements, which comes from feat(workflows): select exact workflow catalog releases #4788 and is unchanged here. The test builds the same ZIP twice with writestr(), which stamps the current time; builds in different 2-second windows produce different digests. Reproduced locally; unrelated to this PR.
At 6792bea1, root unknown-step status/error/events and fan-out missing-step projections were directly compared with main c00dc055.
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.
test_replay_restores_fan_out_aliases_from_completed_if and test_resume_restores_completed_fan_out_item_aliases now join the fan-out container instead of per-item aliases (renamed/enlarged); they still assert the reconstructed item aliases in step_results.
test_private_fan_out_aliases_remain_available_to_child_fan_in became test_composed_fan_out_joins_container_results; test_fan_in_rejects_non_item_fan_out_alias became test_fan_in_rejects_fan_out_item_alias, now covering valid-looking fan:template:0 as well.
test_fan_out_saves_once_per_item_transition and test_tree_backed_resume_has_no_setup_checkpoint expect one more save per started occurrence (2 * items + 6 and 4), because the active occurrence is now checkpointed before it runs, as on main.
test_aborted_fanout_sibling_is_never_restarted is parametrized over step, if-container, and workflow-call templates and also asserts the aborted item's output; the no-restart assertion is unchanged.
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. The pre-existing main behaviors listed under "Review rounds" are also left for follow-ups.
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. The reporting-only fan-out alias change (reverting the fan_out_aliases allowance, adding the FanInStep runtime guard, rewriting the affected tests, and the accompanying docs) was implemented with deepseek-v4.1-flash (opencode-go/deepseek-v4.1-flash), including automated verification and commit. 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.
Later review rounds, all acting autonomously on @markuswondrak's behalf:
1b0a3a46, 3e5e66e8, e5a4223b, 8e9c5fa4, 4db5a079: OpenCode with claude-opus-5.5 (github-copilot/claude-opus-5.5). This includes analysis, comparisons with main, regression tests, automated verification, commit/push, review replies, and this AI-drafted description update.
Each commit names its agent in an Assisted-by: or Co-authored-by: trailer. 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)
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)
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.
Keep fan-out item aliases in the enclosing workflow context without exposing child workflow internals in root results. Normalize unnamed templates to the item ID for result aliases and lifecycle events, and recognize generated item aliases during fan-in validation.
Assisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Review round summary for @markuswondrak: addressed both open findings in 6b776ce3 (fix(workflows): preserve composed fan-out aliases).
Fan-out item aliases now remain in the enclosing child workflow context for downstream fan-in, while private child aliases remain excluded from root state.step_results. Validation accepts only numeric item aliases produced by a preceding fan-out and retains rejection coverage for invalid aliases.
Unnamed fan-out templates now normalize to item before traversal, so results, callbacks, and lifecycle events consistently use fan:item:<index>.
Added regression coverage for composed fan-out to fan-in, private root-result isolation, invalid aliases, and unnamed-template result/event/callback correlation.
AI disclosure: Posted on behalf of @markuswondrak. OpenCode, model github-copilot/gpt-5.6-terra, autonomous mode with user-directed scope, implemented the fixes and regression tests, ran the stated automated checks, and drafted this comment. Human manual or line-by-line review is not attested.
json.dumps() succeeding does not mean the value survives the checkpoint unchanged: for example, a YAML message {1: one} is accepted here but reloads as {"1": "one"}, changing the gate message between run and status/resume. Check a strict JSON round trip (also rejecting NaN) before deciding to preserve the typed value; otherwise store the textual form as intended.
1b0a3a4 checkpointed the active occurrence before step_started, but only
on the shared step path. Workflow calls return to Execution.workflow()
before that path and emitted step_started without a checkpoint. Until
binding was saved, and on resume of an already-bound call until the first
child step, state.json named the previous step, and a crash or checkpoint
failure could leave a start event for the call that no checkpoint
reflected.
Move the start sequence (set current_step_id, checkpoint, then log and
callback) into one Execution.start() helper used by both paths, so the
rule lives in one place. The ID update and checkpoint now happen under one
lock hold. Tests cover top-level, nested, and resumed bound calls, and a
failed start checkpoint that must leave no start event; all three failed
before this change.
Assisted-by: OpenCode (model: claude-opus-5.5, autonomous)
Fixed: workflow calls announced before being checkpointed (r4145873823). This was a gap in 1b0a3a46: that fix saved the active step only on the shared step path, and calls return to Execution.workflow() before reaching it. The start sequence (set current_step_id, checkpoint, then log and callback) now lives in one Execution.start() helper used by both paths. New tests cover top-level, nested, and resumed already-bound calls, plus a failed start checkpoint that must leave no start event; all three failed on 1b0a3a46.
Deferred as pre-existing on main (67ab049e): the remaining findings reproduce unchanged on main with the same script, so this PR keeps the existing behavior. They are tracked in markuswondrak#4.
Concurrent fan-out worker exception dropped after an earlier halt: r4146765980.
Nested if/switch steps losing the fan-out/loop qualifier: r4146766407.
Gate message round-trip (the "previously missed" item at step/gate/__init__.py:49): main stores the message unchanged and checkpoints it with plain json.dump, so {1: one} also reloads as {"1": "one"} there. The check added in this PR only stopped values that could not be saved at all (for example dates) from crashing the checkpoint; the strict round-trip belongs with the follow-up.
Verification: this worktree's .venv/bin/python -m pytest tests/test_workflows.py tests/workflows tests/specify_cli/workflows -q -p no:cacheprovider: 1,447 passed, 1 skipped. uvx ruff@0.15.0 check src tests and git diff --check pass. The full repository suite was not rerun.
Posted on behalf of @markuswondrak by OpenCode (model: claude-opus-5.5 / github-copilot/claude-opus-5.5, autonomous mode with user-directed scope). Analysis, fix, tests, and this comment are AI-generated; findings were reproduced against main and this branch with a script.
RunState.save() caught BaseException around the checkpoint write, so a
KeyboardInterrupt during any checkpoint became CheckpointError, marked the
state as failed to checkpoint, and bypassed the KeyboardInterrupt handler
in execute()/resume(). The run stayed "running" and could not be resumed.
On main the interrupt reaches that handler and the run pauses. Introduced
in 7aa424d.
Convert only Exception to CheckpointError. Write failures still stop
further writes; interrupts propagate unchanged, and the pause re-saves the
tree from memory. Tests interrupt a checkpoint at the root, inside a
workflow call, and during resume; all failed before this change.
Assisted-by: OpenCode (model: claude-opus-5.5, autonomous)
Fixed: interrupts during a checkpoint no longer strand the run (r4146954550). This regression came from 7aa424dd: RunState.save() caught BaseException, so a KeyboardInterrupt during a checkpoint became CheckpointError and skipped the pause handler in execute()/resume(). The same repro on main (67ab049e) pauses and resumes; this branch raised and left the run running, so it could not be resumed. save() now converts only Exception. Write failures still stop further writes, and interrupts reach the pause path, which re-saves the tree from memory. New tests interrupt a checkpoint at the root, inside a workflow call, and during resume, and check that the resumed run doesn't re-run completed steps; all three failed on 3e5e66e8. The existing OSError checkpoint-failure tests are unchanged.
Verification: this worktree's .venv/bin/python -m pytest tests/test_workflows.py tests/workflows tests/specify_cli/workflows -q -p no:cacheprovider: 1,450 passed, 1 skipped. uvx ruff@0.15.0 check src tests and git diff --check pass. The full repository suite was not rerun.
Posted on behalf of @markuswondrak by OpenCode (model: claude-opus-5.5 / github-copilot/claude-opus-5.5, autonomous mode with user-directed scope). Analysis, fix, tests, and this comment are AI-generated; the regression was reproduced against main and this branch with a script.
Replay stored result for already-aborted fan-out items
src/specify_cli/workflows/_execution.py:479
An already-aborted occurrence is returned without replaying its stored result. In the concurrent scenario covered by test_aborted_fanout_sibling_is_never_restarted, item 1 can abort while item 0 pauses; after item 0 completes on resume, replay reaches item 1 here, but run_item() sees no local projection and substitutes {} into the fan-out's ordered results. Project the stored result before returning so exact resume preserves the aborted item's output/alias (and add an output assertion to that regression).
6792bea made fan-out items publish only the result their traversal
projected, so missing step implementations publish nothing. The replay of
an already-aborted occurrence returned without projecting its stored
result, so after a sibling item resumed, the aborted item appeared as {}
in the fan-out's ordered results.
Project the stored result before returning "aborted", as the live run did
when the occurrence aborted. Nothing is re-executed and no events are
emitted. The aborted-sibling regression now checks the item output for a
step, an if container, and a workflow call template; all three failed
before this change.
Assisted-by: OpenCode (model: claude-opus-5.5, autonomous)
Fixed: aborted fan-out items lost their output on replay (_execution.py:479). The regression came from 6792bea1: fan-out items publish only the result their traversal projected, and replaying an already-aborted occurrence returned without projecting its stored result. When a parallel fan-out paused on item 0 while item 1 aborted, resume turned item 1 into {} in results; on 6792bea1~1 it was preserved. The replay now projects the stored result before returning aborted, as the live run did. It still doesn't re-execute the item or emit events, and missing step implementations (outcome failed) are unaffected. test_aborted_fanout_sibling_is_never_restarted now also checks item 1's output for step, if-container, and workflow-call templates; all three failed on e5a4223b.
Verification: this worktree's .venv/bin/python -m pytest tests/test_workflows.py tests/workflows tests/specify_cli/workflows -q -p no:cacheprovider: 1,452 passed, 1 skipped. uvx ruff@0.15.0 check src tests and git diff --check pass. The full repository suite was not rerun.
Posted on behalf of @markuswondrak by OpenCode (model: claude-opus-5.5 / github-copilot/claude-opus-5.5, autonomous mode with user-directed scope). Analysis, fix, tests, and this comment are AI-generated; the regression was bisected to 6792bea1 with a script.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Resumed fan-out items can observe stale partial parent results that were unavailable during initial execution.
Review effort: Balanced Findings: None
Previously missed (1)
In code that hasn't changed since last review
Hide checkpointed fan-out results from resumed item context
src/specify_cli/workflows/_execution.py:823
On resume of an incomplete fan-out, context.steps already contains the parent node's checkpointed partial output.results (projected at lines 567-568), so this snapshot exposes those partial results to every re-executed or not-yet-started item. On the initial execution, items see the fan-out record before results is added. A template that reads steps.fan.output.results therefore changes behavior after a pause and can consume a stale prefix. Remove the engine-added results field from the item-local snapshot (while retaining it in the parent/reporting view), and add a pause/resume regression proving item context is identical.
A halted fan-out checkpoints its partial results in its record for
reporting. On resume that record is projected before the items restart, so
re-executed and not-yet-started items saw steps.<fan>.output.results with
a stale prefix, including the paused item's own earlier output. Items in a
live run, and on main, never see results: the engine adds them only after
the items finish. Present since 7aa424d.
Remove the engine-added results from the fan-out's own entry in the
item-local snapshot. The parent context and checkpoint keep them.
Add an invariant test: a resumed item must see exactly the context it has
in a run that never paused, for step, if-container, and workflow-call
templates, sequentially and in parallel. It found only this difference;
the four affected cases failed before this change.
Assisted-by: OpenCode (model: claude-opus-5.5, autonomous)
Review round for #pullrequestreview-5370347785, addressed in 4db5a079. This covers the previously missed finding from that review.
Fixed: resumed fan-out items saw partial results (_execution.py:823). This was present since 7aa424dd. A halted fan-out checkpoints its partial results for reporting, and resume projected that record before restarting items. With items [0, 1, 2] and item 1 pausing, items 1 and 2 saw steps.fan.output.results == [{item 0}, {item 1}] on resume; on main (67ab049e) and in a live run they never see results. The item-local snapshot now drops the engine-added results from the fan-out's own entry; the parent view and checkpoint keep them.
Guard against further gaps of this kind:test_resumed_fan_out_item_sees_its_uninterrupted_context checks that each item re-executed after a resume sees the same StepContext (excluding run_id/is_resume) as in a run that never paused, for step, if-container, and workflow-call templates, sequentially and in parallel. On 8e9c5fa4 it failed in the four non-call cases, and results was the only difference; workflow-call children have a private context and already matched. test_resumed_fan_out_items_do_not_see_partial_results covers the specific finding and checks that the paused fan-out still reports its partial results. This test covers item inputs only, not published outputs, which test_aborted_fanout_sibling_is_never_restarted covers since the last round.
Verification: this worktree's .venv/bin/python -m pytest tests/test_workflows.py tests/workflows tests/specify_cli/workflows -q -p no:cacheprovider: 1,459 passed, 1 skipped; the new tests passed 15 of 15 repeated runs. uvx ruff@0.15.0 check src tests and git diff --check pass. The full repository suite was not rerun.
Posted on behalf of @markuswondrak by OpenCode (model: claude-opus-5.5 / github-copilot/claude-opus-5.5, autonomous mode with user-directed scope). Analysis, fix, tests, and this comment are AI-generated; the behavior was compared against main and earlier branch commits with a script.
Report included workflow status after interruption or failure
src/specify_cli/workflows/_execution.py:287
An interruption or exception escaping an included step bypasses workflow() finalization: the engine marks the root run paused or failed, but this bound call has no result yet. This fallback therefore reports the included workflow as running in both JSON and human status even though no executor is running. Derive the active unfinished scope's status from the run lifecycle (or persist the boundary outcome) so interrupted scopes report paused and exceptional scopes report failed.
raise ValueError("Invalid execution state: root sequence differs from workflow snapshot")
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
triage-can-waitVerdict: valid and in-scope but deprioritized; held behind the evidence gate
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mnriem
added
the
triage-can-wait
Summary
Reimplements workflow composition from main (originally
c00dc055;upstream/mainhas since been merged, so the merge base is now2c0a57ab) 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:
4db5a079.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 (
2c0a57ab) is 20 files, 4,860 additions / 600 deletions, including 1,269 production additions / 492 deletions undersrc/. 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
fan:template:index) and the orderedfan.output.resultsremain available. This addresses the parallel-item collision in feat(workflows): compose installed workflows via a scoped workflow step #4724.Architecture and contracts
_execution.pyowns tree construction/validation, traversal/replay, control flow, workflow scope entry, projections, and event emission.composition.pyowns target resolution, strict binding, declared outputs, andCallError.engine.pykeeps public lifecycle, input coercion/merge, and RunState persistence.inside_fan_outare transitive. Child gates require explicit root-to-child verdict mapping.continue_on_errorcan 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 resolverRuntimeErroris not converted into a call failure.PAUSEDandFAILEDruns can resume. The earlier crash-resume claim is withdrawn:RUNNINGcheckpoints are rejected. No ownership/lease mechanism is added. External effects before their completion checkpoint remain at-least-once.current_step_id, qualified as in events) before its start is logged and before it executes. Both step paths share oneExecution.start()sequence, asmaindoes in_execute_steps.KeyboardInterrupt), including one raised during a checkpoint, is not a checkpoint failure: it reaches the pause path as onmain, which re-saves the tree from memory. Fan-out aliases are reconstructed projections, set under the existing run lock and saved by the next checkpoint.resultsof a halted fan-out stay in the reporting view only), and an already-aborted sibling replays its stored result instead of publishing{}.fan:template:index) are reporting-only, notfan-in.wait_fortargets.wait_foragain requires declared step IDs (the fan-out step's ownid; ordered item results atsteps.<id>.output.results), matchingmain. Thefan_out_aliasesvalidation allowance is reverted, andFanInStep.execute()rejects:entries at runtime. This closes a stale-read path: in a loop iteration ≥ 1 the authored item alias was never refreshed, so a per-item join silently read the previous iteration's values.Evidence
test_concurrent_nested_calls_keep_downstream_aliases_localwas run against #4724 commit7ece7a16via 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 reporting-only change is covered by
test_fan_in_rejects_fan_out_item_alias(validation) andtest_fan_in_rejects_item_alias_at_runtime(unvalidatedexecute), plustest_composed_fan_out_joins_container_resultsandtest_fan_in_container_join_in_loop_sees_current_iterationas container-join controls. A loop with differing per-iteration values previously let a per-item join read iteration 0; the construct is now unexpressible.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 withc00dc055now produces only the fan-out container result, with matching per-item failure events.Review rounds since
9375b86eFor
3e5e66e8,e5a4223b,8e9c5fa4, and4db5a079, the new tests were shown to fail before the change. "Compared withmain" means the same script was run against67ab049e, whose workflow execution code matches the merge base2c0a57abapart from workflow-version validation.3c503260: no log writes after a checkpoint failure; an unknown child step is a reported call failure.f2dca00f: resume snapshot check covered with native YAML scalars (.nan); no production change.97874f3c,2c0e1415:current_step_idis recomputed on exception paths and uses qualified occurrence IDs; gate messages keep typed values; a fan-out worker exception stops further dispatch.1b0a3a46,3e5e66e8: the active occurrence is checkpointed before it starts;3e5e66e8extends this to workflow calls, which1b0a3a46missed because calls take a separate path.e5a4223b: fixes a regression from7aa424ddwhereRunState.save()caughtBaseException, so aKeyboardInterruptduring a checkpoint left the runrunningand not resumable.mainpauses.8e9c5fa4: fixes a regression from6792bea1where a parallel fan-out's already-aborted item lost its output ({}) when a sibling resumed.4db5a079: resumed fan-out items no longer see the halted fan-out's partialresults(present since7aa424dd;mainnever exposes them).test_resumed_fan_out_item_sees_its_uninterrupted_contextchecks the general invariant for step,if-container, and workflow-call templates, sequentially and in parallel;resultswas the only difference it found.Reported in review, reproduced unchanged on
main, and deferred (tracked in markuswondrak#4): a concurrent fan-out worker exception can be dropped after an earlier item halts; steps nested inif/switchinside a fan-out or loop lose the occurrence qualifier; gate messages that are not lossless JSON (for example{1: one}) change after reload.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,459 passed, 1 skipped.uvx ruff@0.15.0 check src testsandgit diff --check: pass.e5a4223b: ruff, Lint, CodeQL, Security Audit, and Extension Version Guard passed. Onepytest (macos-latest, 3.14)job failed ontests/specify_cli/workflows/test_catalog_versions.py::test_exact_add_uses_historical_url_digest_and_requirements, which comes from feat(workflows): select exact workflow catalog releases #4788 and is unchanged here. The test builds the same ZIP twice withwritestr(), which stamps the current time; builds in different 2-second windows produce different digests. Reproduced locally; unrelated to this PR.Historical evidence and remaining limitation
6792bea1, root unknown-step status/error/events and fan-out missing-step projections were directly compared with mainc00dc055.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.Intentionally changed tests
test_checkpoint_failure_never_overwrites_committed_progressbecametest_checkpoint_failure_leaves_running_run_not_resumable; crash-resume expectations were removed/inverted whenRUNNINGresume was withdrawn.test_rebind_failure_has_one_failed_caller_outcomenow asserts propagation, runFAILED, and an unchanged call node rather than a recoverable call failure.test_output_failure_retries_only_finalizationinjectsCallErrorfor a contract violation.test_output_expression_failure_retries_only_finalizationseparately covers propagating expression errors without repeating children.execute()while retaining their behavioral coverage.test_replay_restores_fan_out_aliases_from_completed_ifandtest_resume_restores_completed_fan_out_item_aliasesnow join the fan-out container instead of per-item aliases (renamed/enlarged); they still assert the reconstructed item aliases instep_results.test_private_fan_out_aliases_remain_available_to_child_fan_inbecametest_composed_fan_out_joins_container_results;test_fan_in_rejects_non_item_fan_out_aliasbecametest_fan_in_rejects_fan_out_item_alias, now covering valid-lookingfan:template:0as well.test_fan_out_saves_once_per_item_transitionandtest_tree_backed_resume_has_no_setup_checkpointexpect one more save per started occurrence (2 * items + 6and4), because the active occurrence is now checkpointed before it runs, as onmain.test_aborted_fanout_sibling_is_never_restartedis parametrized over step,if-container, and workflow-call templates and also asserts the aborted item's output; the no-restart assertion is unchanged.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. The pre-existing
mainbehaviors listed under "Review rounds" are also left for follow-ups.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. The reporting-only fan-out alias change (reverting thefan_out_aliasesallowance, adding theFanInStepruntime guard, rewriting the affected tests, and the accompanying docs) was implemented with deepseek-v4.1-flash (opencode-go/deepseek-v4.1-flash), including automated verification and commit. Intermediate cleanup commits disclose gpt-5.6-terra and deepseek-v4.1-flash individually in theirAssisted-by:trailers. The original rewrite and its AI-assisted #4724 history are retained.Later review rounds, all acting autonomously on @markuswondrak's behalf:
3c503260,f2dca00f: GitHub Copilot (model claude-opus-5.5).2c0e1415: GitHub Copilot CLI (model claude-opus-5.5).97874f3c: Copilot Autofix.1b0a3a46,3e5e66e8,e5a4223b,8e9c5fa4,4db5a079: OpenCode with claude-opus-5.5 (github-copilot/claude-opus-5.5). This includes analysis, comparisons withmain, regression tests, automated verification, commit/push, review replies, and this AI-drafted description update.Each commit names its agent in an
Assisted-by:orCo-authored-by:trailer. Human line-by-line review or manual testing is not attested.