Implement control-flow routing: on_false, on_success, on_failure (#333)

[jeremymanning]

Closes #333. Stacked on #442 — the routing tests assert skipped steps through the typed result, so merge that first.

What was actually missing

condition: already evaluated and already skipped a step. on_false and on_success did not exist anywhere in src/. So the gap was purely routing: where execution goes once the answer is known.

Routing jumps forward and marks the steps in between as skipped, reusing _skip_tasks_between — the machinery goto already had — rather than adding a second one beside it.

Key	Fires when
on_false	the step's own condition: was false, so it was skipped
on_success	the step ran and succeeded
on_failure	the step ran and did not succeed

Skipping is not failing: a pipeline that routed around a step still reports success, and PipelineResult separates skipped_steps from failed_steps accordingly.

The on_failure collision

on_failure already meant a failure policy — fail / continue / skip / retry — and #333 asks for the same key to name a step. Both readings are useful, so both are supported, disambiguated by value: a reserved policy word keeps its policy meaning, anything else is a step id, and when it names a step the run continues there instead of aborting.

That is only safe because the genuinely ambiguous case is refused rather than guessed. A pipeline containing a step called retry fails to compile — on_failure: retry could not be told apart from selecting the policy. core/routing.py holds the single definition; the compiler and the executor both consume it.

Two things I got wrong first, both instructive

I put the compile-time validation in the wrong validator. compiler/schema_validator.py has a validate_dependencies() that nothing calls — the compile path uses validation/dependency_validator.py. My tests failed while the code read correctly. The check now lives in the live path only; the duplicate was reverted rather than left as a second implementation. What did have to stay in schema_validator.py is the schema widening: on_failure was constrained to enum: [continue, fail, retry, skip], which would have rejected every routing target outright.

Routing on TaskStatus sent a failing step down the success path. A tool returning {"success": False} without raising leaves its task COMPLETED, so status is FAILED never matched and on_failure routing silently did nothing — the never step ran and wrote its file. The runtime now asks StepResult.success, reusing the distinction #442 had to draw rather than restating it a third time.

That is the same defect class for the third time in this sequence (#442's exit code, #441's error recording, here). It is worth stating plainly: status answers "did it finish", success answers "did it work", and anything that branches on the wrong one produces a silently wrong result rather than an error.

Compile-time validation

• a jump to a step that does not exist → exit 2, naming the target
• a step routing to itself → exit 2
• a step id colliding with a policy word → exit 2, explaining the ambiguity

Contract updated

ADR 0001 gains a control-flow section, and docs/actions.md is regenerated with the routing table and the policy list read from FAILURE_POLICIES, so the published vocabulary cannot drift from the code. The docs drift test covers it.

Numbers

	before	after
Blocking suite	490 passed	506 passed, 13 skipped, 0 failed, 0 xfailed
Collection	3285	3301
Examples validating	3 / 111	3 / 111
ruff / uv lock --check / git diff --check / docs drift	—	clean

16 new routing tests: each key taken and not taken, the skip representation, the side effect of a routed-over step not happening, policy-vs-step disambiguation across all four policy words, and all three compile-time refusals.

Not in this PR

#320 also bundles experts:, selection_schema: and personality: — model-selection features that have nothing to do with control flow and would make this PR two unrelated changes. #335 (vars: structured outputs) is likewise separate. Both want their own PRs against the now-explicit contract.

[jeremymanning]

CI 9/9 green. The legacy tally moved, and the two tests responsible are flaky, not regressions — with evidence, since the aggregate alone could not tell the difference.

	base (#442)	this PR
failed	473	475
passed	1847	1845
errors	189	189
deselected	589	605

deselected +16 is this PR's routing tests, excluded from the legacy selection by construction. The +2 failed / −2 passed is:

tests/orchestrator/quality/reporting/test_analytics.py::TestAnalyticsResult::test_analytics_result_creation
tests/test_service_manager.py::TestDockerServiceManager::test_is_running_real

Neither is anywhere near control flow. Their history across recent runs:

Run	test_analytics_result_creation	test_is_running_real
#440	failed	passed
#441	failed	failed
#442	passed	passed
#443 (this)	failed	failed

Both pass locally on this branch when run directly. They flip across PRs that touched nothing related to them — including #441, which predates every line of this change.

This corrects something I claimed on #440

I said there that two consecutive main runs being byte-identical (468/1846/195 twice) meant a ±2 swing "would have been real signal". That inference was too strong. Two agreeing samples do not establish determinism, and these two tests demonstrate the legacy suite is not deterministic: its run-to-run noise floor is at least ±2.

The practical consequence for future comparisons: an aggregate delta of ±2 in this suite is not by itself evidence of anything. The named -rfE lists — which #440 added precisely for this — are what make the question answerable, and diffing them is what turned "two tests regressed" into "these two named tests, which also failed two PRs ago".

Worth filing the two as flaky separately; they are pre-existing and unrelated to routing.