ADR-0192: A phase's declared condition gates its execution — three states, never two

Date: 2026-08-01

Renumbered 0171 → 0192 on 2026-08-23. This ADR was authored on a branch that merged into an already-consumed branch and never reached master (#1160), so 0171 was taken in the meantime by Ship docling on the CPU-only torch build. ADR numbers are an append-only log and are never reused (s4u-adr §3), so the late-arriving record takes the next free number rather than the one it was written under. Content is unchanged.

Status: Accepted Deciders: Adrian Cockx (Soft4U BV), Claude Opus 5 (implementation + analysis)

Decision context:

• Latency: negligible and not measured beyond that. The gate is one call to the existing pure evaluate_condition over an in-memory dict, per phase, in a process that already walks every phase; the load-time reachability check is O(phases × referenced ids) once at engine construction. No I/O, no allocation of consequence.
• Dependency surface: none. No new package, no new import outside the module; dynamic_workflow.py stays the dependency-free leaf ADR-0113/#953 made it.
• Debuggability: a withheld phase names itself in three places — a WARNING log line stating which phase and why, a typed entry in state.gaps (reason="condition_unknown") or state.skipped_phases (reason="condition_false"), and state.error on the BLOCKED path. A skipped phase is legible in the state without reading a log. The failure mode this ADR closes was the opposite: an action fired and left no trace that it should not have.
• Reversibility: hours. The gate is one guarded block at the top of execute_phase plus one elif branch in the engine loop; deleting both restores the prior behaviour exactly. No migration, no persisted schema change, no flag — see "Alternatives Considered" for why no flag.
• Blast radius: DynamicWorkflowEngine has no production consumers. It is imported by tests/test_dynamic_workflow.py and named in a doc-comment in app/api/workflow_schemas.py. It is absent from app/worker.py and app/main.py. Within the module the change is additive for every phase that declares no condition (all of vendor_due_diligence_v1, and 4 of 6 phases in kyb_onboarding_v1); it is substitutive only for the 3 gated ACTION phases across the two schemas that declare one.
• Alternative considered: keep the two-state model and treat a FALSE gate as a not_executed gap. Rejected — it reports missing scrutiny that is not missing, so every approved case would carry a permanent "rejection_handling did not execute" gap and every rejected case the mirror image. A gap register that fills with expected entries stops being read.

Context

WorkflowPhase.condition has existed since the declarative engine was written. It is parsed by the Pydantic model, stored on the phase, and — since #953 — validated structurally at engine construction by validate_schema_conditions. It was never evaluated. execute_phase dispatched on phase.type alone; no branch read phase.condition.

kyb_onboarding_v1.yaml declares two mutually exclusive ACTION phases, both depends_on: [compliance_review], distinguished only by their conditions: activation (decision in [approve]) and rejection_handling (decision equals reject). Both are in the compiled ExecutionPlan, so both ran on every case. Measured on the real shipped YAML, driven through the real engine, on the code as it stood immediately before this change:

=== REJECTED CASE (sanctions_match=True, score=99) ===
  review decision        : 'reject'
  ACTION phases EXECUTED : ['activation', 'rejection_handling']
      performed -> activation:create_entity          <- the Customer record
      performed -> activation:schedule_review
      performed -> activation:activate_pkyc          <- monitoring enabled
      performed -> activation:notification           <- onboarding_approved
      performed -> rejection_handling:notification

A rejected subject was onboarded: Customer record created, perpetual-KYC monitoring switched on, and the customer told they were approved — followed by a rejection notice. periodic_review_v1.yaml has the same shape on review_completion.

An action gated on approval firing on a rejection is a control doing the opposite of its intent. It is the ADR-0067 failure in its most direct form: the system did not merely fail to add scrutiny, it performed the clearing effect on a subject it had itself decided to reject.

The defect is latent: the engine has no production consumers (see Blast radius). That is why it is being fixed now — wiring a runner without fixing it ships it.

Three questions had to be answered before the fix could be written, and #953 deliberately scoped them out rather than half-answer them inside a remediation PR:

1. How is a deliberately-skipped phase represented? It must be distinguishable from a completed one and from #953's not_executed gap, which means "could not run" and blocks the workflow. Two states cannot carry three meanings.
2. What happens when the gate is UNKNOWN — the condition's data is absent? The issue notes the safe answer differs by intent: never activate on unverified state; but a notification-only phase might reasonably still fire.
3. Can the gate discriminate at all on the human path? The auto-decision path writes decision into the review phase's result, so phases.<review>.decision resolves. The officer path did not: resume() wrote {"status": "signal_received"} and nothing else, leaving every gate UNKNOWN.

Decision

A phase that declares a condition executes only when that condition evaluates TRUE. A phase that declares none is untouched. The gate is evaluated in execute_phase, before any type dispatch, via a new pure evaluate_phase_condition(phase, context) -> PhaseGate reusing #953's three-valued evaluate_condition — the same evaluator, the same Kleene semantics, the same UnsupportedConditionError. No second condition language is introduced.

1. Four outcomes, three recorded states

Gate	Runs?	Status returned	Recorded in
absent	yes	(unchanged)	completed_phases
TRUE	yes	(unchanged)	completed_phases
FALSE	no	skipped_by_condition	skipped_phases
UNKNOWN	no	not_executed (reason="condition_unknown")	gaps, workflow → BLOCKED
uninterpretable	no	raises UnsupportedConditionError	workflow → FAILED

PhaseGate.declared is the load-bearing field. "Declares no condition" and "condition could not be answered" are both "we have no TRUE", and conflating them would withhold every unconditional phase in every schema. They are separated before anything is evaluated.

skipped_phases is a new list on DynamicWorkflowState, disjoint from completed_phases (the phase did not happen) and from gaps (nothing is missing — the question was asked and answered). Its entries carry assessed: True, deliberately: a designed skip is not absent scrutiny, and filing it as a gap would report a deficiency that does not exist.

completed_phases is a record, not an index — resume() seeks on _step_index — so adding a third list has no effect on resume positioning.

2. UNKNOWN withholds and blocks, uniformly across phase types

An UNKNOWN gate means the workflow cannot establish whether the phase should run. It does not run, and the workflow goes BLOCKED with a typed condition_unknown gap.

The direction is uniform rather than per-action-type policy. Withholding an action removes an effect that can be applied later; running one on unverified state (creating a Customer record, enabling monitoring, telling a customer they are approved) is not reversible by the engine. And blocking is not a dead end: #953's rule 5 already RETRIES a blocked phase on resume(), so the phase re-evaluates its gate once the missing data arrives, rather than being consumed. That composition is what makes the uniform rule affordable — a per-action-type policy would have to be invented, justified, and maintained without evidence for any of its entries.

3. A resumed review phase carries its decision verb — and only that

inject_signal now binds the signal to the phase that is paused when it is injected (state.signals_by_phase), and resume() copies only the decision value from that bound signal onto a REVIEW phase's phase view, alongside the existing status: signal_received. Nothing else from the payload enters the condition namespace.

Binding per-phase rather than reading the flat context["officer_decision"] key matters because that key is overwritten by each new signal: a schema with two review phases would otherwise complete the second with the first's stale decision. A stale approval activating a customer is the same defect class this ADR closes, so the fix must not reintroduce it one layer down.

Without this, every officer decision leaves phases.<review>.decision absent, every downstream gate is UNKNOWN, and the gate blocks every human-reviewed case instead of discriminating between them. A gate that blocks everything is not a gate — it is the false-alarm mode that gets a fail-closed control switched back off.

4. A gate that can never be answered is refused at load

validate_schema_conditions already checks that a phases.<id> reference names a phase that exists. Existing is not sufficient: a gate reading a phase that runs at or after the gated phase can never be answered, so under rule 2 it would block every case forever. _validate_phase_gate_reachability refuses such a schema at engine construction, mirroring _validate_route_targets. Both shipped gated schemas load unchanged, pinned by a control test.

Consequences

Positive

• A rejected subject is no longer onboarded. Measured, on the shipped YAML, on both the auto-decision and officer paths.
• Two mutually exclusive terminal phases can no longer both execute on one case.
• state now distinguishes ran / deliberately did not run / could not run, so an audit surface can render a never-run activation as such rather than as a performed one.
• A standing contract test walks every phase of every shipped schema that declares a condition and proves the runner is not invoked when the gate is not TRUE. A gated phase added tomorrow is covered without anyone remembering to add a test — the discovery mechanism, not the instance.
• An unanswerable gate is a load-time error with a named phase, not a per-case block discovered in production.

Negative

• A phase whose depends_on names a skipped phase still runs. The engine walks the compiled plan linearly and does not re-check dependencies at runtime; that predates this change and is not fixed here. No shipped schema exercises it (both gated phases are plan leaves), and deciding the right behaviour — cascade the skip, or treat the dependency as satisfied — needs a case that does not exist yet. Recorded rather than guessed at. Tracked as a follow-up.
• UNKNOWN blocks the whole workflow, not just the gated phase. On a schema where a later phase is independent of the unanswerable one, that later phase is also withheld. This is the deliberate fail-closed direction, but it is stricter than strictly necessary, and on a schema with many gated phases it will surface one blocker at a time rather than all of them.
• skipped_by_condition is a fourth status a Temporal wrapper must map, on top of the pause, blocked and failed statuses. The wrapper does not exist yet, so this is a cost deferred to whoever writes it — named here so it is not a surprise.
• state.signals_by_phase grows with the number of paused phases and holds the raw signal payload, which for an officer decision may carry free text. It is engine state, not persisted to a store today, but a Temporal wrapper persisting the state across Continue-As-New will be persisting that payload.
• One existing test's assertion was loosened from whole-dict equality on a resumed review result to per-key assertions, because the result now legitimately carries decision.

Neutral

• No migration, no configuration flag, no new dependency.
• Behaviour for every phase that declares no condition is unchanged, including the whole of vendor_due_diligence_v1.yaml.
• The engine remains unwired from production; this changes what it will do when it is wired, not what any live case does today.

Alternatives Considered

Alternative 1: Dark-launch the gate behind a settings flag

• The repository's usual pattern (ADR-0146) for a behaviour-changing control: merge with default=False, flip after a Calibration Review.
• Why rejected: a flag's purpose is to protect production traffic while a control is validated. There is no production traffic — the engine has no consumers. A flag would therefore protect nothing while guaranteeing that the engine, when wired, is wired in its defective state unless someone remembers to flip it. ADR-0146 exists because exactly that happened to ~23 other controls. The fix restores the schema author's declared intent; shipping it off by default would ship the defect.

Alternative 2: Per-phase-type (or per-action-type) UNKNOWN policy

• Withhold on UNKNOWN for a phase performing create_entity/activate_pkyc, but allow a notification-only phase to proceed — the asymmetry the issue itself names.
• Why rejected: it requires classifying every action verb as clearing or not, which is a registry that must be complete to be safe and has no evidence behind any entry today. The fail-closed default for an unclassified verb would be "withhold" anyway, so the registry only ever relaxes the rule — and a wrong relaxation fires an unverified action. Revisit when a real schema needs it, with the schema as the evidence.

Alternative 3: Represent a skipped phase as a not_executed gap

• Reuse #953's existing two-state machinery unchanged; no new list, no new status.
• Why rejected: it conflates "the schema said not to run this" with "this could not run", which is precisely the taxonomy conflation the never-suppress doctrine keeps producing (ADR-0094's adverse_media_recall_gap is the same shape). Every approved case would carry a permanent gap for rejection_handling and every rejected one for activation. A gap register whose entries are mostly expected is a register nobody reads — which converts a real gap into an invisible one.

Alternative 4: Do nothing until the engine is wired

• The defect is latent; fix it as part of the wiring work.
• Why rejected: the wiring change is where attention will be on Temporal primitives, signals and Continue-As-New, not on whether a YAML condition is consulted. The issue's own framing — "wiring a runner without fixing this ships the defect" — is the argument. Fixing it now also means the wiring PR inherits the three-state contract rather than inventing a second one.

References

• Issue #954 — the defect report and its execution-confirmed follow-up comment.
• #912 / PR #953 — fail-closed route condition evaluation; this ADR extends the same evaluator to phase conditions and adds rule 8 to that module's contract.
• ADR-0067 — fail-closed compliance outputs; "not assessed" never reads as clear.
• ADR-0094 — the taxonomy-conflation precedent cited in Alternative 3.
• ADR-0146 — the dark-launch pattern, and why it does not apply here.