feat(engine): add type: terminate step (#219)#228
Open
jrob5756 wants to merge 4 commits into
Open
Conversation
Adds a terminal step type that ends the workflow with an explicit
`status` (`success` | `failed`) and a Jinja2-rendered `reason`,
distinguishable from the default `$end` path in the CLI exit code,
JSONL event log, and web dashboard.
Behaviour
- `status: success`: workflow returns normally, CLI exits 0, dashboard ✅,
`workflow_completed { termination_reason, terminated_by, is_explicit: true }`.
- `status: failed`: raises `WorkflowTerminated`, CLI exits 1 (and still
prints the rendered output JSON to stdout), dashboard ❌, dedicated engine
handler emits `workflow_failed { error_type: 'WorkflowTerminated',
is_explicit: true, output, termination_reason, terminated_by }` and
intentionally does NOT save an on-failure checkpoint (explicit termination
is not resumable).
- Optional `output_template: dict[str, str]` replaces the workflow-level
`output:` for that termination path (same shape as workflow output).
- Sub-workflow boundary: failed terminate inside a child sub-workflow is
downgraded to `ExecutionError` so the parent treats it as a normal
sub-workflow failure (parent's own `workflow_failed` does not inherit
`is_explicit: true`).
Schema / validator
- Adds `terminate` to `AgentDef.type` Literal plus `status`, `reason`,
and `output_template` fields, all forbidden on every other step type.
- Rejects `routes`, `tools`, `output`, `prompt`, `model`, etc. on
terminate steps so authoring mistakes fail fast.
- Forbids terminate inside parallel-group members and for_each inline agents
(those contexts wrap/swallow exceptions and would mask the explicit signal).
- Treats terminate as a sink (peer of `$end`) in path enumeration; output
coverage analysis skips terminate paths that supply their own
`output_template`.
- Validates Jinja2 templates inside `reason` and `output_template` values.
Tests / docs
- 8 engine tests, 2 sub-workflow tests, 20 schema tests, 9 validator tests,
3 CLI integration tests, 4 exception tests; full suite (2824 tests) passes.
- `examples/terminate.yaml` demonstrates success + failure paths.
- `AGENTS.md` documents the new step type.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow-up to the initial `type: terminate` implementation, addressing findings from a comprehensive PR review (code, tests, comments, silent-failure, type-design, dead-code). Critical fixes -------------- - `_run_child_engine` now raises a dedicated `SubworkflowTerminatedError` (subclass of `ExecutionError`) carrying the child's rendered `terminated_output` / `terminated_reason` / `terminated_by`. Previously the child's structured `output_template` dict was silently dropped at the parent boundary. - CLI `run` / `resume` handlers now wrap `json.dumps(e.output)` with `default=str` plus a try/except so a future non-serialisable `output_template` value cannot crash the CLI and lose the termination reason; suggestions are now surfaced alongside the reason. Type-design polish ------------------ - Drop the dead `status` parameter on `WorkflowTerminated` (always "failed" in practice); expose `status` as a property. - Make `terminated_by` a property over `agent_name` so the two fields cannot drift apart. - Tighten `output: dict` to `dict[str, Any]`. Engine safety ------------- - Reorder `check_timeout()` before `record_execution()` in the terminate branch so a workflow at `max_iterations` cannot mask explicit termination behind `MaxIterationsError`. - Wrap `_build_terminate_output()` so a template error emits `agent_failed` for the terminate step before re-raising (prevents "stuck agent" state in the dashboard / JSONL log). - Update terminate-branch comment to acknowledge that terminate may be the workflow's `entry_point`. CLI / validator hygiene ----------------------- - Promote the `WorkflowTerminated` import to module scope in `cli/app.py`. - Reframe `except WorkflowTerminated: raise` comments in `cli/run.py` as defense-in-depth (the existing checkpoint-path guard makes them redundant in practice, but the explicit catch protects against future drift in either layer). - Replace `getattr(agent, "type", None) == "terminate"` in `_collect_template_strings` with an `isinstance(agent, AgentDef)` check so a future field-rename surfaces loudly instead of silently skipping template validation. Docs ---- - Document the implicit JSON coercion of rendered `output_template` values in the schema docstring (`"true"` -> True, etc.). Tests (15 new) -------------- - CLI: split stdout/stderr assertions, add `resume` failed-terminate parity test. - Engine: terminate as entry_point, terminate routed from parallel group, terminate routed from for_each group, `on_complete` hook firing, `on_error` hook firing, `agent_failed`-before-`workflow_failed` event ordering. - Sub-workflow: child `output` preservation across boundary, failed-terminate inside for_each-of-workflow (`_with_inputs` path). - Schema: cross-product (status / reason / output_template) x (script / workflow / human_gate) rejection coverage. - All 2839 tests pass; lint clean; typecheck clean (modulo pre-existing dialog_evaluator warning). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comprehensive documentation pass for the new `type: terminate` step. User-facing surfaces updated ---------------------------- - README.md: added Terminate steps to the features list and to the Examples table. - docs/workflow-syntax.md: new "Terminate Steps" section with example, behaviour table (status × CLI exit / dashboard / event / resumability), output-template rendering rules including JSON coercion, restrictions list, and sub-workflow boundary semantics. Updated the `type:` enum comment in the agent example. - docs/configuration.md: added `terminate` to the list of agent types where `reasoning.effort` is rejected (it doesn't call a model). - examples/README.md: added Explicit Termination section with all three paths (success, failed, normal) and `is_explicit: true` hint for CI / observability consumers. Conductor skill (AI-author-facing) updated ------------------------------------------ - plugins/conductor/skills/conductor/SKILL.md: added `type: terminate` row to the Key Concepts table. - plugins/conductor/skills/conductor/references/yaml-schema.md: added `terminate` to the type enum, added Terminate-only fields block, and added the full restrictions paragraph naming WorkflowTerminated / SubworkflowTerminatedError boundary semantics. - plugins/conductor/skills/conductor/references/authoring.md: new "Terminate Steps (`type: terminate`)" section with motivation (multiple legitimate end states beyond $end), full example, semantics, restrictions, and the parent-branching guidance (use `status: success` + `output_template:` if the parent needs to route on a child's outcome). Internal docs / inline updated ------------------------------ - AGENTS.md: expanded the existing one-liner bullet to mention `SubworkflowTerminatedError`, the JSON coercion behaviour, the hook firing contract, and pointers to the new docs. - CHANGELOG.md: Unreleased entry describing the feature in full detail (event payload, sub-workflow downgrade, schema rejection matrix, restrictions, link to issue #219). - src/conductor/config/schema.py: expanded `AgentDef` class docstring to enumerate all five step kinds and point at the per-type validator and the cross-cutting structural rules in validator.py. - tests/test_engine/test_subworkflow.py: ruff format tidy. Validation ---------- - make lint clean, make typecheck clean (modulo pre-existing dialog_evaluator warning), 1295 targeted tests pass, example workflow validates and runs both paths correctly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Closes the last remaining acceptance criterion from the original issue:
"Dashboard shows terminate steps distinctly (e.g. red/green pill with
reason text)". Engine and CLI already emitted all the metadata needed;
this commit teaches the frontend how to consume it.
Graph node
----------
- New `TerminateNode` component (Octagon icon, status-themed border,
body shows "terminate · success" / "terminate · failed" sub-label
plus the rendered reason). Distinct from AgentNode (Bot icon),
ScriptNode (Terminal icon), WorkflowNode, and GateNode so terminate
steps are immediately identifiable in the DAG.
- `NodeType` extended with `'terminate'` and
`graph-layout.ts` maps that to `terminateNode` (was silently
falling through to `agentNode`).
- `WorkflowGraph.tsx` registers `terminateNode` in its
`nodeTypes` map.
Tooltip
-------
- `NodeTooltip` gained optional `reason` and `terminationStatus`
fields. When set, the tooltip shows a green/red "Termination"
capsule row plus a "Reason:" section with the rendered text — so
hovering a terminate node surfaces the full context without having
to open the agent detail panel.
Workflow-level banners
----------------------
- New `workflowTermination` state on the store, populated from the
`is_explicit` / `termination_reason` / `terminated_by` /
`status` fields on the root `workflow_completed` and
`workflow_failed` events. Reset across all replay / load paths.
- `WorkflowSuccessBanner` renders "Workflow Terminated" with the
rendered reason and `terminated_by` line when the workflow ended
via `type: terminate status: success`. Otherwise it keeps the
existing "Completed" rendering verbatim.
- `WorkflowErrorBanner` renders "Workflow Terminated" (instead of
"Workflow Failed") for an explicit-failed termination, plus the
`terminated_by` line. Generic failures keep the existing banner.
Node-level event handlers
-------------------------
- `agent_completed` and `agent_failed` handlers now capture
`termination_reason` / `terminated_by` / `status` from the
event when the engine attached them, storing the values on
`NodeData` so `TerminateNode` and `NodeTooltip` can render
them. Non-terminate agents are unaffected (the fields are simply
absent from their event payloads).
Build artifacts
---------------
Includes the rebuilt `src/conductor/web/static/assets/index-*.{css,js}`
and updated `index.html` — these are tracked in git (see commit
dc29c2c for the convention) so the dashboard bundle ships with the
Python package.
Validation
----------
- npm run build (tsc -b && vite build): clean.
- make lint, make typecheck (modulo pre-existing dialog warning): clean.
- 92 web tests still pass.
This closes acceptance criterion #6 for issue #219.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #219.
Adds a new
type: terminateworkflow step that ends the workflow with an explicitstatus(success|failed) and a structuredreason, distinguishable from the default$endpath in the CLI exit code, dashboard state, and event logs.Motivation
Real workflows have multiple legitimate end states beyond "the last agent finished":
With only
$end, all of these collapse into "workflow completed" in CI exit codes, dashboards, and JSONL logs. Downstream tooling cannot distinguish a clean no-op from a real failure.type: terminatemakes the distinction explicit.Shape
See
examples/terminate.yamlfor a complete worked example.Behaviour
statussuccess0workflow_completed { termination_reason, terminated_by, is_explicit: true, status }failed1workflow_failed { error_type: "WorkflowTerminated", termination_reason, terminated_by, is_explicit: true, status, output }status: failedraisesWorkflowTerminated(subclass ofExecutionError); the CLI prints the rendered output JSON to stdout and surfaces the reason banner on stderr, then exits non-zero.status: failedterminate inside a child sub-workflow is downgraded at the parent boundary toSubworkflowTerminatedError(also anExecutionError), preserving the child's renderedterminated_output/terminated_reason/terminated_byas structured attributes. The parent treats it as a normal sub-workflow failure (its ownworkflow_faileddoes NOT inheritis_explicit: true). Astatus: successchild terminate returns its rendered output cleanly and the parent continues.routes,tools,output,prompt,model, etc.; cannot appear as parallel-group members or as afor_eachinline agent (route to one from those groups'routes:instead). Conversely,status/reason/output_templateare rejected on every non-terminate step type so authors who forgettype: terminateget a clear error rather than silently dropped fields.Acceptance criteria
type: terminateaccepted withstatus,reason, optionaloutput_templateroutes/tools/output/ forbidden fields on terminate stepsstatus:(0 vs non-zero)workflow_completed/workflow_failedevent includestermination_reasonandterminated_byTerminateNodecomponent (Octagon icon, status-themed border, sub-labelterminate · success/terminate · failed, body shows rendered reason);NodeTooltipshows a green/red Termination row plus the full reason on hover;WorkflowSuccessBannerandWorkflowErrorBannerrender "Workflow Terminated" (instead of generic "Completed" / "Failed") with the rendered reason andterminated_byline whenis_explicit: true. See commitec5be86.terminateends just the sub-workflow; parent continues normallyexamples/terminate.yamlImplementation overview
src/conductor/exceptions.pyWorkflowTerminated(ExecutionError)(carriesoutput,reason,terminated_by,status) andSubworkflowTerminatedError(ExecutionError)(carriesterminated_output,terminated_reason,terminated_by)src/conductor/config/schema.pyterminatetoAgentDef.typeLiteral; newstatus,reason,output_templatefields with per-type forbidden-field enforcement (in both directions)src/conductor/config/validator.pyfor_eachinline agents; treat terminate as a sink in routing graph / path enumeration; output-coverage analysis skips terminate paths that supply their ownoutput_template; Jinja2 templates inreasonandoutput_templatevalidated at config-load timesrc/conductor/engine/workflow.py_execute_loop(placed beforehuman_gate/script/workflow); dedicatedexcept WorkflowTerminatedarm that emitsworkflow_failedand skips checkpoint save;_run_child_engine()boundary helper used by both sub-workflow paths converts childWorkflowTerminated→SubworkflowTerminatedError;_build_terminate_output()handles theoutput_template-vs-fallback logic and emitsagent_failedon template render errorssrc/conductor/cli/app.py+cli/run.pyWorkflowTerminatedhandlers in bothrunandresumecommands (split stdout JSON / stderr banner / suggestion); resume-instruction suppression forWorkflowTerminated(defense-in-depth)docs/workflow-syntax.mdTerminate Steps section;examples/README.mdExplicit Termination section;plugins/conductor/skills/conductor/{SKILL.md,references/yaml-schema.md,references/authoring.md};README.mdfeatures + examples table;AGENTS.md;CHANGELOG.md_with_inputspath), CLI (4), exception (4). Full suite: 2839 passed, 16 skipped.Review process
This PR went through a comprehensive review pass after the initial implementation, using six specialised reviewer agents (code, tests, comments, silent-failure, type-design, dead-code). The PR-review-polish commit (
da82f9b) addresses every critical and important finding before this submission:json.dumpsin the CLI handler (default=str+ try/except) so a non-serialisable output value cannot crash the CLI and lose the termination reason.SubworkflowTerminatedErrorso the child's renderedoutput_templateis preserved across the parent boundary as a structured attribute (previously silently dropped).WorkflowTerminated: dropped the deadstatusparameter, madeterminated_bya property overagent_name(eliminates drift), tightenedoutput: dict→dict[str, Any].check_timeout()beforerecord_execution()in the dispatch branch so a workflow atmax_iterationscannot mask explicit termination behindMaxIterationsError._build_terminate_output()so a template error emitsagent_failed(prevents dashboard "stuck agent" state).resumefailed-terminate parity, terminate as entry-point / routed from parallel-group / routed from for_each,on_complete/on_errorhook firing, event ordering, child-output preservation, for_each-of-workflow_with_inputspath, schema cross-product rejection.Commits
35d010efeat(engine): addtype: terminatestepda82f9brefactor(terminate): apply PR-review polishedb58eadocs(terminate): document type: terminate across user-facing docsec5be86feat(web): render type: terminate steps distinctly in dashboardOut of scope (suggested follow-ups)
AgentDef's single-model-for-all-types pattern is now visibly straining (~140 lines of per-type forbidden-field enforcement); worth tracking but explicitly out of scope here for consistency with existing patterns._print_resume_instructionssuppression for all non-resumableConductorErrorsubclasses (currently onlyWorkflowTerminatedskips the hint).🤖 Co-authored with Copilot CLI.