diff --git a/apps/api/openapi.json b/apps/api/openapi.json index 57dd18eb922..4563889f095 100644 --- a/apps/api/openapi.json +++ b/apps/api/openapi.json @@ -3874,6 +3874,14 @@ ], "title": "Actuation Kind" }, + "closing_failures": { + "description": "Every closing step that failed. Always empty on `acquisition_halt=True`. Never flips `succeeded`.", + "items": { + "$ref": "#/components/schemas/ConductorFailureResponse" + }, + "title": "Closing Failures", + "type": "array" + }, "completed_count": { "title": "Completed Count", "type": "integer" @@ -3986,6 +3994,14 @@ ], "title": "Actuation Kind" }, + "closing_failures": { + "description": "Every closing step that failed. Always empty on `held=True`: closing runs only on a real terminal, never on Held. Never flips `succeeded`.", + "items": { + "$ref": "#/components/schemas/ConductorFailureResponse" + }, + "title": "Closing Failures", + "type": "array" + }, "completed_count": { "title": "Completed Count", "type": "integer" @@ -4097,6 +4113,14 @@ ], "title": "Actuation Kind" }, + "closing_failures": { + "description": "Every closing step that failed. Isolated from `failure`: a closing failure never flips `succeeded`, and one closing step failing does not stop the rest of the closing walk. Empty when the recipe has no closing steps, or all ran clean.", + "items": { + "$ref": "#/components/schemas/ConductorFailureResponse" + }, + "title": "Closing Failures", + "type": "array" + }, "completed_count": { "title": "Completed Count", "type": "integer" @@ -4137,7 +4161,7 @@ } ] }, - "description": "Every control address this conduct wrote, in first-write order, carrying the last value written to each. CORA does not restore what it sets, and a halt returns at the failing step without running the recipe's remaining steps, so on a failed conduct this is what was left set with nothing having put it back. Reports what was WRITTEN, not what changed: a write whose value already matched the address still appears.", + "description": "Every control address this conduct wrote, in first-write order, carrying the last value written to each. Includes the recipe's closing steps, which run on a real terminal (Completed or Aborted). Reports what was WRITTEN, not what changed: a write whose value already matched the address still appears.", "title": "Substrate Writes", "type": "object" }, @@ -6645,6 +6669,12 @@ "title": "Capability Id", "type": "string" }, + "closing_steps": { + "additionalProperties": true, + "description": "Optional wire-format closing-step sequence, same shape as `steps`: `{steps: [...]}`. The Conductor walks these after `steps` ends on a real terminal (Completed or Aborted), never on a Held pause. Empty by default; most recipes have none.", + "title": "Closing Steps", + "type": "object" + }, "name": { "description": "Display name for the new Recipe.", "maxLength": 200, @@ -11846,13 +11876,18 @@ "type": "string" }, "RecipeResponse": { - "description": "Read-side DTO at the API boundary.\n\nCarries primitives, not domain VOs. `status` is the StrEnum's\nstring value (Defined / Versioned / Deprecated). `version` is\nthe operator-supplied label of the most recent `version_recipe`\ncall (null until first version). `steps` is the wire-format\ndict (BindingRef sentinels serialize as `{__binding__: name}`).\n`replaced_by_recipe_id` is null on Defined / Versioned /\nDeprecated-without-replacement; populated when a deprecation\nsupplied a successor pointer. `created_at` / `versioned_at` /\n`deprecated_at` are projection-sourced lifecycle timestamps\n(Path C); see module docstring for null semantics.", + "description": "Read-side DTO at the API boundary.\n\nCarries primitives, not domain VOs. `status` is the StrEnum's\nstring value (Defined / Versioned / Deprecated). `version` is\nthe operator-supplied label of the most recent `version_recipe`\ncall (null until first version). `steps` is the wire-format\ndict (BindingRef sentinels serialize as `{__binding__: name}`).\n`replaced_by_recipe_id` is null on Defined / Versioned /\nDeprecated-without-replacement; populated when a deprecation\nsupplied a successor pointer. `created_at` / `versioned_at` /\n`deprecated_at` are projection-sourced lifecycle timestamps\n(Path C); see module docstring for null semantics. `closing_steps`\nmirrors `steps`' wire shape; empty for the common case of a recipe\nwith no closing steps.", "properties": { "capability_id": { "format": "uuid", "title": "Capability Id", "type": "string" }, + "closing_steps": { + "additionalProperties": true, + "title": "Closing Steps", + "type": "object" + }, "created_at": { "anyOf": [ { @@ -11939,6 +11974,7 @@ "status", "version", "steps", + "closing_steps", "replaced_by_recipe_id" ], "title": "RecipeResponse", @@ -17230,6 +17266,12 @@ "VersionRecipeRequest": { "description": "Body for `POST /recipes/{recipe_id}/version`.", "properties": { + "closing_steps": { + "additionalProperties": true, + "description": "Replacement closing-step sequence, same shape as `steps` (wholesale replace alongside it). Empty by default.", + "title": "Closing Steps", + "type": "object" + }, "steps": { "additionalProperties": true, "description": "Replacement step sequence for the new version (wholesale replace; the prior steps are dropped). BindingRef sentinels are re-validated against the CURRENT Capability.parameters_schema.", @@ -40947,7 +40989,7 @@ "description": "Procedure is not in `Held` status, OR its parent Run is itself `Held` (off-diagonal guard)." }, "422": { - "description": "Path parameter or request body failed schema validation." + "description": "Path parameter or request body failed schema validation, OR a closing step's CaptureRef names a capture only a pre-boundary main step declares (ClosingCaptureBeforeBoundaryError)." }, "500": { "content": { diff --git a/apps/api/src/cora/infrastructure/record_export/_dispositions.py b/apps/api/src/cora/infrastructure/record_export/_dispositions.py index 060bcfdfdff..64eca74b4ae 100644 --- a/apps/api/src/cora/infrastructure/record_export/_dispositions.py +++ b/apps/api/src/cora/infrastructure/record_export/_dispositions.py @@ -1493,6 +1493,21 @@ }, "RecipeDefined": { "capability_id": "token:uuid", + "closing": { + "address": "drop:text", + "capture_name": "drop:text", + "command": "drop:text", + "criterion": "drop:opaque", + "input_uris": "by-value", + "name": "drop:text", + "output_ref_name": "drop:text", + "output_uri": "drop:text", + "parameters": "drop:opaque", + "params": "drop:opaque", + "timeout_s": "keep:number", + "value": "by-value", + "verify": "keep:number", + }, "name": "drop:text", "occurred_at": "keep:time", "recipe_id": "token:uuid", @@ -1532,6 +1547,21 @@ "steps_hash": "drop:text", }, "RecipeVersioned": { + "closing": { + "address": "drop:text", + "capture_name": "drop:text", + "command": "drop:text", + "criterion": "drop:opaque", + "input_uris": "by-value", + "name": "drop:text", + "output_ref_name": "drop:text", + "output_uri": "drop:text", + "parameters": "drop:opaque", + "params": "drop:opaque", + "timeout_s": "keep:number", + "value": "by-value", + "verify": "keep:number", + }, "occurred_at": "keep:time", "recipe_id": "token:uuid", "steps": { @@ -1554,6 +1584,7 @@ "ResolvedStepsRecorded": { "occurred_at": "keep:time", "procedure_id": "token:uuid", + "resolved_closing_steps": "drop:opaque", "resolved_steps": "drop:opaque", "step_count": "keep:number", }, diff --git a/apps/api/src/cora/operation/_conduct_preparation.py b/apps/api/src/cora/operation/_conduct_preparation.py index d68e692dc72..afe450e8608 100644 --- a/apps/api/src/cora/operation/_conduct_preparation.py +++ b/apps/api/src/cora/operation/_conduct_preparation.py @@ -66,6 +66,7 @@ def decide_resolved_steps_recorded( resolved_steps: Sequence[Mapping[str, Any]], *, now: datetime, + resolved_closing_steps: Sequence[Mapping[str, Any]] = (), ) -> list[ResolvedStepsRecorded]: """Pin the resolved step list iff the Procedure is pre-conduct (Defined). @@ -77,16 +78,22 @@ def decide_resolved_steps_recorded( normal lifecycle failure, preserving the conduct route's failures-in-body contract instead of raising a fresh HTTP error here. Kept as a pure function so the decision is unit-testable without an event store. + + `resolved_closing_steps` is the SAME resolution applied to the Recipe's + closing steps, pinned separately (not flattened into `resolved_steps`) + so a resume boundary can never land inside the closing region. """ if state is None or state.status is not ProcedureStatus.DEFINED: return [] steps = tuple(dict(step) for step in resolved_steps) + closing_steps = tuple(dict(step) for step in resolved_closing_steps) return [ ResolvedStepsRecorded( procedure_id=state.id, resolved_steps=steps, - step_count=len(steps), + step_count=len(steps) + len(closing_steps), occurred_at=now, + resolved_closing_steps=closing_steps, ) ] @@ -102,16 +109,22 @@ async def resolve_and_pin_conduct_steps( principal_id: UUID, correlation_id: UUID, causation_id: UUID | None, -) -> tuple[Step, ...]: +) -> tuple[tuple[Step, ...], tuple[Step, ...]]: """Resolve the final conduct step list + pin it as `ResolvedStepsRecorded`. The shared pre-Conductor work for `conduct` / `conduct_or_hold`: recipe re-expansion (recipe-driven Procedures) -> pseudoaxis constituent - expansion (Run-phase Procedures) -> pin. Returns the resolved steps to - hand to the Conductor. `command_name` rides the pinned event's metadata. + expansion (Run-phase Procedures) -> pin. Returns `(steps, closing_steps)` + to hand to the Conductor. `command_name` rides the pinned event's + metadata. + + A legacy (non-recipe-driven) Procedure has no closing steps: `caller_steps` + is an inline list with no separate closing half, so `closing_steps` is + always `()` on that path. """ + closing_steps: tuple[Step, ...] = () if procedure.recipe_id is not None: - steps = await _re_expand_steps( + steps, closing_steps = await _re_expand_steps( procedure_id=procedure.id, recipe_id=procedure.recipe_id, caller_steps=caller_steps, @@ -146,13 +159,21 @@ def _resolve_constituents(asset_id: UUID) -> tuple[UUID, ...]: # Pre-Conductor PseudoAxis expansion: rewrite any virtual-axis SetpointStep # into N sequential constituent SetpointSteps so the Conductor's dispatch # loop walks the constituents in declared order. ActionStep / CheckStep - # pass through unchanged ([[project-pseudoaxis-design]] v3). + # pass through unchanged ([[project-pseudoaxis-design]] v3). Closing steps + # get the SAME expansion, or a pseudoaxis closing setpoint would reach the + # Conductor unresolved. steps = await expansion_port.expand_pseudoaxis( steps, event_store=deps.event_store, correlation_id=correlation_id, constituent_resolver=constituent_resolver, ) + closing_steps = await expansion_port.expand_pseudoaxis( + closing_steps, + event_store=deps.event_store, + correlation_id=correlation_id, + constituent_resolver=constituent_resolver, + ) # Pin the resolved step list (after recipe + pseudoaxis expansion) BEFORE # conducting, so a future resume replays this exact list. The helper emits @@ -163,6 +184,7 @@ def _resolve_constituents(asset_id: UUID) -> tuple[UUID, ...]: procedure, tuple(step_to_payload(step) for step in steps), now=deps.clock.now(), + resolved_closing_steps=tuple(step_to_payload(step) for step in closing_steps), ) if resolved_steps_events: _, current_version = await deps.event_store.load( @@ -187,7 +209,7 @@ def _resolve_constituents(asset_id: UUID) -> tuple[UUID, ...]: ], ) - return steps + return steps, closing_steps async def _re_expand_steps( @@ -198,7 +220,7 @@ async def _re_expand_steps( stored_events: list[StoredEvent], event_store: EventStore, expansion_port: RecipeExpander, -) -> tuple[Step, ...]: +) -> tuple[tuple[Step, ...], tuple[Step, ...]]: """Run the recipe-replay gate per [[project-run-procedure-replay-design]]. Six steps: reject non-empty caller steps -> find_recipe_expansion_record @@ -209,7 +231,8 @@ async def _re_expand_steps( propagates from helper) -> load_capability + reject Deprecated (raise ProcedureBoundCapabilityDeprecatedError, symmetric to start_run's RunBoundPlanDeprecatedError) -> verify_bindings_hash -> - expand -> verify_steps_hash -> return the re-expanded tuple. + expand (both `recipe.steps` and `recipe.closing_steps`) -> + verify_steps_hash (one combined pin) -> return `(steps, closing_steps)`. """ if list(caller_steps): raise ProcedureStepsForbiddenForRecipeDrivenError(procedure_id) @@ -244,6 +267,8 @@ async def _re_expand_steps( raise ProcedureBoundCapabilityDeprecatedError(procedure_id, recipe.capability_id) verify_bindings_hash(procedure_id, pins) - expanded = expansion_port.expand(recipe.steps, dict(pins.bindings)) - verify_steps_hash(procedure_id, expanded, pins) - return expanded + bindings_dict = dict(pins.bindings) + expanded = expansion_port.expand(recipe.steps, bindings_dict) + expanded_closing = expansion_port.expand(recipe.closing_steps, bindings_dict) + verify_steps_hash(procedure_id, expanded, pins, closing_steps=expanded_closing) + return expanded, expanded_closing diff --git a/apps/api/src/cora/operation/_conduct_wire.py b/apps/api/src/cora/operation/_conduct_wire.py index 6ed2d4f00ee..4ce11aeb919 100644 --- a/apps/api/src/cora/operation/_conduct_wire.py +++ b/apps/api/src/cora/operation/_conduct_wire.py @@ -22,7 +22,7 @@ tuple for the in-process Conductor. """ -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from typing import Annotated, Any, Literal, cast from pydantic import BaseModel, Field @@ -192,3 +192,15 @@ def substrate_writes_to_wire( `result_to_wire` (conduct / conduct_or_hold / conduct_from) now that a third call site would otherwise repeat it.""" return {k: list(v) if isinstance(v, tuple) else v for k, v in substrate_writes.items()} + + +def closing_failures_to_wire( + closing_failures: Sequence[ConductorFailure], +) -> list[ConductorFailureResponse]: + """Project `ConductorResult.closing_failures` onto its JSON wire shape. + + Reuses `failure_to_wire` per entry -- a closing failure has the exact + same shape as the main-walk `failure`, just isolated into its own list + rather than halting the walk. Empty by default: most conducts have no + closing steps at all, or every closing step ran clean.""" + return [failure_to_wire(f) for f in closing_failures] diff --git a/apps/api/src/cora/operation/_recipe_expansion/__init__.py b/apps/api/src/cora/operation/_recipe_expansion/__init__.py index 501ba47062b..970e5ededca 100644 --- a/apps/api/src/cora/operation/_recipe_expansion/__init__.py +++ b/apps/api/src/cora/operation/_recipe_expansion/__init__.py @@ -7,7 +7,8 @@ that replay / verify that resolution: - `_expand`: the pure `expand(steps, bindings) -> Step list` substitution kernel - (+ `steps_to_wire` / `canonical_json_bytes` for provenance hashing). + (+ `steps_to_wire` / `steps_to_wire_with_closing` / `canonical_json_bytes` + for provenance hashing). - `_replay`: locate + verify the genesis `RecipeExpansionRecorded` provenance on the `conduct_procedure` path (re-expand and compare pinned hashes). - `_resolved_steps_replay`: locate the pinned `ResolvedStepsRecorded` provenance @@ -21,6 +22,7 @@ canonical_json_bytes, expand, steps_to_wire, + steps_to_wire_with_closing, ) from cora.operation._recipe_expansion._replay import ( MismatchField, @@ -43,6 +45,7 @@ "find_resolved_steps_record", "pins_from_payload", "steps_to_wire", + "steps_to_wire_with_closing", "verify_bindings_hash", "verify_steps_hash", ] diff --git a/apps/api/src/cora/operation/_recipe_expansion/_expand.py b/apps/api/src/cora/operation/_recipe_expansion/_expand.py index 9928410f668..00a254f0d7e 100644 --- a/apps/api/src/cora/operation/_recipe_expansion/_expand.py +++ b/apps/api/src/cora/operation/_recipe_expansion/_expand.py @@ -229,4 +229,27 @@ def steps_to_wire(steps: tuple[Step, ...]) -> list[dict[str, Any]]: return [_step_to_wire(step) for step in steps] -__all__ = ["canonical_json_bytes", "expand", "steps_to_wire"] +def steps_to_wire_with_closing( + steps: tuple[Step, ...], closing_steps: tuple[Step, ...] +) -> list[dict[str, Any]]: + """Canonical hash form for main + closing steps COMBINED, one pin. + + Closing entries are tagged `"closing": true` by THIS composing + function, not inside `_step_to_wire` (which receives a bare `Step` + with no partition context). An empty `closing_steps` returns + EXACTLY `steps_to_wire(steps)` with nothing appended, so every + recipe that has never used closing steps keeps its existing + `steps_hash` byte-identical -- same precedent as `CheckStep.timeout_s` + being emitted only when set (see + `test_a_deadlineless_check_hashes_the_same_as_it_always_did`). + """ + main = steps_to_wire(steps) + if not closing_steps: + return main + closing = steps_to_wire(closing_steps) + for entry in closing: + entry["closing"] = True + return main + closing + + +__all__ = ["canonical_json_bytes", "expand", "steps_to_wire", "steps_to_wire_with_closing"] diff --git a/apps/api/src/cora/operation/_recipe_expansion/_replay.py b/apps/api/src/cora/operation/_recipe_expansion/_replay.py index 14bd64666cc..2c548d17cf1 100644 --- a/apps/api/src/cora/operation/_recipe_expansion/_replay.py +++ b/apps/api/src/cora/operation/_recipe_expansion/_replay.py @@ -24,7 +24,7 @@ from uuid import UUID from cora.infrastructure.ports.event_store import StoredEvent -from cora.operation._recipe_expansion._expand import steps_to_wire +from cora.operation._recipe_expansion._expand import steps_to_wire_with_closing from cora.operation.aggregates.procedure import ( RecipeExpansionRecordNotFoundError, RecipeExpansionReplayMismatchError, @@ -121,6 +121,8 @@ def verify_steps_hash( procedure_id: UUID, steps: tuple[Step, ...], pins: RecipeExpansionPins, + *, + closing_steps: tuple[Step, ...] = (), ) -> None: """Verify the re-expanded steps still hash to the recorded `steps_hash`. @@ -129,8 +131,16 @@ def verify_steps_hash( produces different output for the same input than at write time); runs AFTER `verify_bindings_hash` because steps drift downstream of bindings is a confusing diagnostic. + + `closing_steps` composes into the SAME pinned hash (one pin, no + aliasing): `steps_hash` was written over `steps_to_wire_with_closing` + at registration time, so replay must recompute the same combined + form. Defaults to `()`, which reproduces the pre-closing-steps hash + exactly for every recipe that has never used the field. """ - recomputed = hashlib.sha256(canonical_json_bytes(steps_to_wire(steps))).hexdigest() + recomputed = hashlib.sha256( + canonical_json_bytes(steps_to_wire_with_closing(steps, closing_steps)) + ).hexdigest() if recomputed != pins.steps_hash: raise RecipeExpansionReplayMismatchError(procedure_id, "steps") diff --git a/apps/api/src/cora/operation/aggregates/procedure/events.py b/apps/api/src/cora/operation/aggregates/procedure/events.py index c90a8e6dbc8..6b74119a05a 100644 --- a/apps/api/src/cora/operation/aggregates/procedure/events.py +++ b/apps/api/src/cora/operation/aggregates/procedure/events.py @@ -590,6 +590,13 @@ class ResolvedStepsRecorded: rewrite. `step_count` is a denorm for cheap read-side checks, mirror of `RecipeExpansionRecorded.step_count`. + `resolved_closing_steps` is the SAME resolution applied to the + Recipe's `closing_steps`, kept in a SEPARATE field rather than + flattened onto `resolved_steps`: a flat pin would let an + operator-supplied `conduct_from` boundary land inside the closing + region, and `execute_from` would replay closing steps as main steps + at absolute indices. Empty by default (additive, optional). + Provenance-only: the evolver leaves Procedure state unchanged when this event arrives (mirrors `RecipeExpansionRecorded`). """ @@ -598,6 +605,7 @@ class ResolvedStepsRecorded: resolved_steps: tuple[Mapping[str, Any], ...] step_count: int occurred_at: datetime + resolved_closing_steps: tuple[Mapping[str, Any], ...] = () # Discriminated union of every event the Procedure aggregate emits. @@ -882,12 +890,14 @@ def to_payload(event: ProcedureEvent) -> dict[str, Any]: resolved_steps=resolved_steps, step_count=step_count, occurred_at=occurred_at, + resolved_closing_steps=resolved_closing_steps, ): return { "procedure_id": str(procedure_id), "resolved_steps": [dict(step) for step in resolved_steps], "step_count": step_count, "occurred_at": occurred_at.isoformat(), + "resolved_closing_steps": [dict(step) for step in resolved_closing_steps], } case _: # pragma: no cover # exhaustiveness guard assert_never(event) @@ -1143,6 +1153,11 @@ def _build_resumed() -> ProcedureResumed: resolved_steps=tuple(dict(step) for step in payload["resolved_steps"]), step_count=int(payload["step_count"]), occurred_at=datetime.fromisoformat(payload["occurred_at"]), + # `.get`, not `["resolved_closing_steps"]`: a stored event + # from before closing steps existed has no such key. + resolved_closing_steps=tuple( + dict(step) for step in payload.get("resolved_closing_steps", []) + ), ), ) case _: diff --git a/apps/api/src/cora/operation/conductor.py b/apps/api/src/cora/operation/conductor.py index bfaef71ce16..63e8bdb371f 100644 --- a/apps/api/src/cora/operation/conductor.py +++ b/apps/api/src/cora/operation/conductor.py @@ -967,18 +967,33 @@ class ConductorResult: """Every address this conduct wrote, in first-write order, last value. Present on the success and the failure construction alike, because a - conduct changes the world either way. It earns its place on the - FAILURE one: a halt returns from the step loop, so a recipe's own - closing steps do not run, and this is then the list of what was left - set with nothing having put it back. Without it an operator has to - reconstruct that from the step journal at exactly the moment they - are least able to. + conduct changes the world either way. On a TERMINAL outcome (Completed + or Aborted) this INCLUDES whatever the recipe's own closing steps wrote: + it is no longer only "what was left set with nothing having put it + back" -- a closing-only address now appears here too, since closing + steps write through the SAME `_ActuationObserver`. On a Held pause + (closing does not run) it is still exactly that: the list of what CORA + left set, unrestored, because a later `conduct_from` resumes against + this exact state. Without it an operator has to reconstruct that from + the step journal at exactly the moment they are least able to. It reports what CORA WROTE, not what it changed: a write whose value already matched the PV still appears. Deciding otherwise would mean reading every address back, which is a second round of substrate traffic to answer a question the operator did not ask. """ + closing_failures: tuple[ConductorFailure, ...] = () + """Every closing step that failed, in walked order; empty on success. + + Closing steps are ISOLATED from each other and from `failure`: one + failing is recorded and the next still runs, and a closing failure + never changes `succeeded` (that bit reports the MAIN steps only, + per [[project_conduct_closing_steps_design]]). Empty whenever closing + did not run at all (Held, an acquisition halt, cancellation, a + Procedure with no `closing_steps`) -- indistinguishable from "closing + ran and every step passed"; a caller that needs to tell the two apart + reads the recipe's `closing_steps` list length instead. + """ @property def succeeded(self) -> bool: @@ -1050,10 +1065,16 @@ def substrate_writes(self) -> Mapping[str, WriteValue]: CORA does not restore what it sets (see `acquisitions`' prior / applied recording for why a tidy-up write would clobber a - concurrent client), so on a HALT this is the list of things left - as CORA put them, with the recipe's own closing steps unrun. An - operator reading a failed conduct should not have to reconstruct - that from the step journal. + concurrent client). A wrapper that runs closing steps (`conduct`, + `conduct_or_hold`'s terminal arms, `conduct_from`'s terminal arms) + merges `_run_closing`'s own observer's writes on top of this map + before returning the final result -- on a TERMINAL outcome this is + no longer only "what was left with nothing having put it back"; it + also includes what closing put back. On a HELD pause (closing does + not run) it is still exactly the pre-closing meaning: the list of + things left as CORA put them. An operator reading a conduct's + result should not have to reconstruct either case from the step + journal. Reads are deliberately absent: reading changes nothing, and mixing them in would bury the writes that actually need @@ -1246,6 +1267,7 @@ async def execute_from( policy: ResumePolicy = ResumePolicy.RE_ESTABLISH, causation_id: UUID | None = None, surface_id: UUID = NIL_SENTINEL_ID, + captures: dict[str, Any] | None = None, ) -> ConductorResult: """Resume a halted conduct by REPLAYING the pinned resolved steps from `boundary`. @@ -1293,7 +1315,11 @@ async def execute_from( # boundary is not replayed, so a CaptureRef into it fails loud rather # than resolving against stale data. Persisting captures across a hold # (seed this dict from a ValueCaptured event) is the deferred resume leg. - captures: dict[str, Any] = {} + # `captures` is caller-owned like `execute()`'s (None creates a fresh + # dict): `conduct_from` passes one in so `_run_closing` can share + # whatever the replay tail deposits. + if captures is None: + captures = {} # The artifact bus is likewise empty on resume and never filled here: a # ComputeStep reached during replay halts for an operator decision (it is # never dispatched), so nothing deposits and no OutputRef resolves against @@ -1375,6 +1401,88 @@ async def execute_from( outputs=dict(outputs), ) + async def _run_closing( + self, + closing_steps: Sequence[Step], + *, + procedure_id: UUID, + principal_id: UUID, + correlation_id: UUID, + causation_id: UUID | None, + surface_id: UUID, + captures: dict[str, Any], + ) -> tuple[tuple[ConductorFailure, ...], Mapping[str, WriteValue]]: + """Walk `closing_steps` after a conduct reaches a REAL terminal. + + Called by `conduct`, `conduct_or_hold` (its two terminal arms only: + complete-success and abort), and `conduct_from` (its complete and + abort arms only) -- never on a Held pause, an acquisition halt, or + cancellation. See [[project_conduct_closing_steps_design]] for the + full six-row disposition table this enforces. + + ISOLATED per step, unlike the main list: one step failing is + recorded and the WALK CONTINUES; this method itself NEVER raises, + so it cannot mask whatever the caller does next (return a result, + or re-raise an already-in-flight exception). Runs in its own + `step_index` space starting at 0 (see `_Envelope.closing`). + + `captures` is the conduct's OWN captures bus, passed in by + reference and shared with the main list (a closing step MAY read a + value the main list captured; the reverse is never true since + closing runs after). Returns `(closing_failures, substrate_writes)` + for the caller to fold onto its `ConductorResult`: closing gets its + own fresh `_ActuationObserver`, so its writes are NOT already + reflected in the main list's `substrate_writes` and the caller + must merge them explicitly. + + An empty `closing_steps` is the common case (no Recipe authors + one): short-circuits before touching the ControlPort at all. + """ + if not closing_steps: + return (), {} + envelope = _Envelope( + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + causation_id=causation_id, + surface_id=surface_id, + closing=True, + ) + observer = _ActuationObserver(self._control_port) + outputs: dict[str, ArtifactRef] = {} + compute = _ComputeAccumulator() + failures: list[ConductorFailure] = [] + for index, step in enumerate(closing_steps): + try: + with with_dispatch_correlation_id(correlation_id): + failure = await self._dispatch( + step, + index=index, + envelope=envelope, + port=observer, + captures=captures, + outputs=outputs, + compute=compute, + ) + except Exception as exc: + # "Never raises" is a GUARANTEE, not an intention: a closing + # step can raise for the same reasons a main step's dispatch + # can (a buggy action body, an unexpected port error class), + # and closing is exactly the walk that must survive one bad + # step to still attempt the rest. Convert to a recorded + # failure instead of propagating. + failure = ConductorFailure( + step_index=index, + source_kind=_closing_step_kind(step), + target=_closing_step_target(step), + error_class=type(exc).__name__, + message=str(exc), + ) + if failure is not None: + failures.append(failure) + # ISOLATED: no halt-on-failure. The next closing step always runs. + return tuple(failures), observer.substrate_writes + async def conduct( self, *, @@ -1384,6 +1492,7 @@ async def conduct( steps: Sequence[Step], causation_id: UUID | None = None, surface_id: UUID = NIL_SENTINEL_ID, + closing_steps: Sequence[Step] = (), ) -> ConductorResult: """Drive the full Procedure lifecycle: start -> execute -> complete | abort. @@ -1404,6 +1513,23 @@ async def conduct( (the Procedure stays Running and the operator must reconcile via state inspection). + `closing_steps` runs via `_run_closing` on a REAL terminal only: once + `execute()` reaches Completed-bound success or a step failure, and + BEFORE the corresponding `complete_procedure` / `abort_procedure` + call -- closing's own journal writes go through `append_activities`, + which accepts entries only while the Procedure is `Running` + (`_OPEN_STATUSES = {RUNNING}`); attempting them after the FSM has + already left Running would reject every one of them. So the terminal + FSM write is genuinely LAST here, not first: closing physically runs + (and its outcome is durably journaled) regardless of whether the + SUBSEQUENT `complete_procedure` call itself is rejected. Also runs + before the best-effort abort on an unhandled exception from + `execute()` (the literal TomoScan shape this design cites -- see the + outer `except Exception` below). NOT run on cancellation + (`abort_orphan_on_cancel` already re-raises before any return here). + See [[project_conduct_closing_steps_design]] for the full + disposition table. + Requires `start_procedure` + `complete_procedure` + `abort_procedure` handlers to have been supplied at __init__. Raises `RuntimeError` if any of the three is missing; this @@ -1454,23 +1580,72 @@ async def conduct( # then re-raises so the caller's task still sees the cancellation. No # ConductorResult exists on cancellation, so the observed kind is # unrecoverable and the abort records None (a Dataset off a cancelled - # conduct carries no proven kind). + # conduct carries no proven kind). Cancellation does NOT run closing: + # a cancellation is typically loop teardown, the worst moment to + # start driving hardware, and this re-raises before any return below. abort_procedure = self._abort_procedure - async with abort_orphan_on_cancel( - lambda: abort_procedure( - AbortProcedure(procedure_id=procedure_id, reason="cancelled mid-execute"), - **envelope_kwargs, - ) - ): - result = await self.execute( + captures: dict[str, Any] = {} + try: + async with abort_orphan_on_cancel( + lambda: abort_procedure( + AbortProcedure(procedure_id=procedure_id, reason="cancelled mid-execute"), + **envelope_kwargs, + ) + ): + result = await self.execute( + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + steps=steps, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + except asyncio.CancelledError: + raise + except Exception: + # The literal TomoScan shape: execute() deliberately lets a + # non-port exception propagate (a programmer error, not a + # ConductorFailure), and closing must still run before it + # leaves -- BEFORE the best-effort abort, so closing's own + # journal writes land while the Procedure is still Running (see + # the docstring). Runs for its RECORDING side-effect alone here, + # since there is no ConductorResult to attach closing_failures + # to when the caller is about to see this exception, not a + # return value. + with contextlib.suppress(Exception): + await self._run_closing( + closing_steps, + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + with contextlib.suppress(Exception): + await abort_procedure( + AbortProcedure( + procedure_id=procedure_id, reason="unhandled exception during execute" + ), + **envelope_kwargs, + ) + raise + if result.succeeded: + closing_failures, closing_writes = await self._run_closing( + closing_steps, procedure_id=procedure_id, principal_id=principal_id, correlation_id=correlation_id, - steps=steps, causation_id=causation_id, surface_id=surface_id, + captures=captures, + ) + result = replace( + result, + closing_failures=closing_failures, + substrate_writes={**result.substrate_writes, **closing_writes}, ) - if result.succeeded: try: await self._complete_procedure( CompleteProcedure( @@ -1492,8 +1667,10 @@ async def conduct( except Exception as exc: # `replace`, not a fresh ConductorResult: a hand-copied # field list silently drops whatever the caller forgets to - # list, and `substrate_writes` is exactly the field an - # operator needs on a completion that itself failed. + # list. Closing already ran (above) and its ledger survives + # here even though complete_procedure itself was REJECTED -- + # closing is what physically happened; the FSM label is a + # separate, subsequent fact that failed to record. return replace( result, failure=ConductorFailure( @@ -1505,11 +1682,26 @@ async def conduct( ), ) return result - # execute failed; attempt abort with a derived reason. Best-effort: - # if abort itself fails, surface the original step failure since - # that is what the caller needs to triage. + # execute failed. Closing runs BEFORE the best-effort abort (its + # journal writes need the Procedure still Running); abort itself is + # best-effort regardless: if it fails, surface the original step + # failure since that is what the caller needs to triage. failure = result.failure assert failure is not None # not result.succeeded implies failure + closing_failures, closing_writes = await self._run_closing( + closing_steps, + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + result = replace( + result, + closing_failures=closing_failures, + substrate_writes={**result.substrate_writes, **closing_writes}, + ) reason = _derive_failure_reason(failure) with contextlib.suppress(Exception): await self._abort_procedure( @@ -1535,6 +1727,7 @@ async def conduct_or_hold( steps: Sequence[Step], causation_id: UUID | None = None, surface_id: UUID = NIL_SENTINEL_ID, + closing_steps: Sequence[Step] = (), ) -> ConductorResult: """Drive the lifecycle like `conduct()`, but PAUSE to Held on a recoverable failure. @@ -1555,6 +1748,18 @@ async def conduct_or_hold( - lifecycle failures (start / complete rejected) and a mid-execute `CancelledError` keep `conduct()`'s behavior verbatim (no hold). + `closing_steps` runs on exactly TWO of the arms above: complete- + success, and the non-recoverable-failure abort -- and runs BEFORE + the corresponding `complete_procedure` / `abort_procedure` call, for + the same reason as `conduct()`: closing's own journal writes need + the Procedure still Running. It does NOT run on Held (a hold is a + pause whose resume replays `steps[boundary:]` against the state the + pre-boundary steps established; closing first would tear that + down), on a failed hold attempt (the Procedure is left Running, not + terminal), or on cancellation. A subsequent complete-rejection does + NOT undo a closing walk that already ran. See + [[project_conduct_closing_steps_design]]. + Requires `start_procedure` + `complete_procedure` + `abort_procedure` + `hold_procedure` handlers at __init__; raises `RuntimeError` (a wiring bug) otherwise. @@ -1601,23 +1806,67 @@ async def conduct_or_hold( # Mirror conduct(): a mid-execute cancellation best-effort aborts so the # FSM is not orphaned in Running, then re-raises. A cancellation is not a # recoverable step failure, so it aborts rather than pausing to Held. + # Closing does NOT run here either, for the same reason as conduct(). abort_procedure = self._abort_procedure - async with abort_orphan_on_cancel( - lambda: abort_procedure( - AbortProcedure(procedure_id=procedure_id, reason="cancelled mid-execute"), - **envelope_kwargs, - ) - ): - result = await self.execute( + captures: dict[str, Any] = {} + try: + async with abort_orphan_on_cancel( + lambda: abort_procedure( + AbortProcedure(procedure_id=procedure_id, reason="cancelled mid-execute"), + **envelope_kwargs, + ) + ): + result = await self.execute( + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + steps=steps, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + except asyncio.CancelledError: + raise + except Exception: + # Mirror conduct()'s raised-exception handling: closing runs + # BEFORE the best-effort abort (its journal writes need the + # Procedure still Running), for its recording side-effect alone + # (no ConductorResult to attach closing_failures to when + # re-raising). + with contextlib.suppress(Exception): + await self._run_closing( + closing_steps, + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + with contextlib.suppress(Exception): + await abort_procedure( + AbortProcedure( + procedure_id=procedure_id, reason="unhandled exception during execute" + ), + **envelope_kwargs, + ) + raise + actuation_kind = result.actuation_kind.value if result.actuation_kind is not None else None + if result.succeeded: + closing_failures, closing_writes = await self._run_closing( + closing_steps, procedure_id=procedure_id, principal_id=principal_id, correlation_id=correlation_id, - steps=steps, causation_id=causation_id, surface_id=surface_id, + captures=captures, + ) + result = replace( + result, + closing_failures=closing_failures, + substrate_writes={**result.substrate_writes, **closing_writes}, ) - actuation_kind = result.actuation_kind.value if result.actuation_kind is not None else None - if result.succeeded: try: await self._complete_procedure( CompleteProcedure(procedure_id=procedure_id, actuation_kind=actuation_kind), @@ -1626,6 +1875,8 @@ async def conduct_or_hold( except _LIFECYCLE_RERAISE: raise except Exception as exc: + # Closing already ran (above) and survives a SUBSEQUENT + # complete_procedure rejection; see conduct()'s twin comment. return replace( result, failure=ConductorFailure( @@ -1661,13 +1912,31 @@ async def conduct_or_hold( if held_ok: # A Held Procedure is parked mid-flight awaiting an operator # decision, which is exactly when `substrate_writes` has to - # survive: it is the list of what CORA left set, and the - # recipe's own closing steps did not run. + # survive: it is the list of what CORA left set. NO closing: + # a hold is a pause, not a terminal; a later conduct_from + # resumes against the world the main steps established, and + # closing first would tear that down. return replace(result, failure=failure, held=True) return result # Non-recoverable step failure (action): best-effort abort, exactly # like conduct(). Holding would strand a Procedure whose replay tail - # starts with an interrupted acquisition. + # starts with an interrupted acquisition. Closing runs BEFORE the + # abort attempt (its journal writes need the Procedure still + # Running), regardless of whether the abort itself then succeeds. + closing_failures, closing_writes = await self._run_closing( + closing_steps, + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + result = replace( + result, + closing_failures=closing_failures, + substrate_writes={**result.substrate_writes, **closing_writes}, + ) with contextlib.suppress(Exception): await self._abort_procedure( AbortProcedure( @@ -2707,6 +2976,7 @@ async def conduct_from( prior_actuation_kind: str | None = None, causation_id: UUID | None = None, surface_id: UUID = NIL_SENTINEL_ID, + closing_steps: Sequence[Step] = (), ) -> ConductorResult: """Resume a Held Procedure and REPLAY its pinned resolved steps from `boundary`. @@ -2744,6 +3014,19 @@ async def conduct_from( Running with partial replay history, the same posture as the acquisition-halt branch (the operator reconciles). See [[project_resumable_conduct_design]] Tier 1. + + `closing_steps` runs via `_run_closing` on exactly TWO of the three + terminals: the clean-tail complete, and the genuine-step-failure + abort -- and runs BEFORE the corresponding `complete_procedure` / + `abort_procedure` call, same reason as `conduct()`: closing's own + journal writes need the Procedure still Running. NOT on the + acquisition halt (the Procedure stays Running, not terminal -- + closing there would foreclose the very redo-fresh-vs-reseed + decision being handed back), and NOT on a raised exception that is + itself a cancellation (mirrors this method's existing + no-abort-on-cancel posture). A subsequent complete-rejection does + NOT undo a closing walk that already ran. See + [[project_conduct_closing_steps_design]]. """ if ( self._resume_procedure is None @@ -2768,15 +3051,39 @@ async def conduct_from( ResumeProcedure(procedure_id=procedure_id, re_establishment_boundary=boundary), **envelope_kwargs, ) - result = await self.execute_from( - procedure_id=procedure_id, - principal_id=principal_id, - correlation_id=correlation_id, - steps=steps, - boundary=boundary, - causation_id=causation_id, - surface_id=surface_id, - ) + captures: dict[str, Any] = {} + try: + result = await self.execute_from( + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + steps=steps, + boundary=boundary, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + except asyncio.CancelledError: + raise + except Exception: + # Mirror conduct()'s raised-exception handling for the literal + # TomoScan shape. Unlike conduct(), no best-effort abort precedes + # it here -- this method already leaves a mid-replay cancellation + # un-aborted by design (the operator reconciles), and the same + # posture applies to any other raised exception: the Procedure + # stays Running, closing runs for its recording side-effect + # alone, then the exception propagates unchanged. + with contextlib.suppress(Exception): + await self._run_closing( + closing_steps, + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + raise # Fold the pre-hold conduct's kind (carried on the Held procedure, # passed in by the handler) with the replay tail's observed kind, so a # boundary>0 resume past a simulated prefix does not complete as @@ -2793,8 +3100,24 @@ async def conduct_from( actuation_kind=(ActuationKind(actuation_kind) if actuation_kind is not None else None), ) if result.succeeded: - # Clean tail (incl. empty tail): auto-complete, threading the - # merged observed kind onto ProcedureCompleted (Data BC gate carrier). + # Clean tail (incl. empty tail): closing runs BEFORE the + # auto-complete attempt (its journal writes need the Procedure + # still Running), threading the merged observed kind onto + # ProcedureCompleted (Data BC gate carrier). + closing_failures, closing_writes = await self._run_closing( + closing_steps, + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + merged_result = replace( + merged_result, + closing_failures=closing_failures, + substrate_writes={**merged_result.substrate_writes, **closing_writes}, + ) try: await self._complete_procedure( CompleteProcedure(procedure_id=procedure_id, actuation_kind=actuation_kind), @@ -2806,7 +3129,8 @@ async def conduct_from( # `merged_result`, not `result`: the merged kind is what the # terminal event carried, so the response has to agree with it # on the complete-rejected arm exactly as it does on the - # success arm below. + # success arm below. Closing already ran (above) and survives + # this SUBSEQUENT rejection; see conduct()'s twin comment. return replace( merged_result, failure=ConductorFailure( @@ -2820,6 +3144,9 @@ async def conduct_from( return merged_result if is_acquisition_halt(result.failure): # Halt-for-operator: leave the Procedure Running; no transition. + # NO closing: the Procedure is not terminal, and closing here + # would foreclose the redo-fresh-vs-reseed decision being handed + # back to the operator. # RESIDUAL: the replay tail's observed kind is NOT persisted here # (no terminal event), so a later manual complete/abort -- which # SETs actuation_kind from the command, not merges -- could stamp @@ -2827,10 +3154,26 @@ async def conduct_from( # this method closes; the design-memo second-writer hazard, aligned # with the Tier-2 acquisition-decomposition deferral. return merged_result - # Genuine step failure: best-effort abort (if abort itself fails, the - # original step failure is what surfaces). Mirrors conduct(). + # Genuine step failure: closing runs BEFORE the best-effort abort + # (its journal writes need the Procedure still Running); abort + # itself is best-effort regardless (if it fails, the original step + # failure is what surfaces). Mirrors conduct(). failure = result.failure assert failure is not None # not succeeded + not halt -> failure + closing_failures, closing_writes = await self._run_closing( + closing_steps, + procedure_id=procedure_id, + principal_id=principal_id, + correlation_id=correlation_id, + causation_id=causation_id, + surface_id=surface_id, + captures=captures, + ) + merged_result = replace( + merged_result, + closing_failures=closing_failures, + substrate_writes={**merged_result.substrate_writes, **closing_writes}, + ) with contextlib.suppress(Exception): await self._abort_procedure( AbortProcedure( @@ -4146,13 +4489,18 @@ async def _record( `index` is the step's zero-based position in the conducted step list; it rides the payload as `step_index` so a future resume can map a recorded outcome back to its position in the pinned - resolved step list. + resolved step list. `envelope.closing` (`_run_closing`'s walk) + adds a `"closing": true` marker: closing runs in its OWN + `step_index` space starting at 0, so the index alone would + collide with a main step's row. """ payload: dict[str, Any] = {**body, "step_index": index, "result": result} if error_class is not None: payload["error_class"] = error_class if message is not None: payload["message"] = message + if envelope.closing: + payload["closing"] = True sampled_at = self._clock.now() entry = ActivityInput( event_id=self._id_generator.new_id(), @@ -4176,6 +4524,13 @@ class _Envelope: Internal helper; avoids passing six args to every helper method. Frozen so accidental mutation mid-execute is a type error. + + `closing` marks a `_run_closing` walk: `_record` tags the journal row + `"closing": true` when set, disambiguating a closing step's outcome + from a main step's in the SAME activity log (closing runs in its own + `step_index` space starting at 0, so the index alone does not + disambiguate). Threaded via the envelope rather than a `_record` + kwarg so none of its other call sites need touching. """ procedure_id: UUID @@ -4183,6 +4538,7 @@ class _Envelope: correlation_id: UUID causation_id: UUID | None surface_id: UUID + closing: bool = False @dataclass @@ -4441,6 +4797,29 @@ def _derive_failure_reason(failure: ConductorFailure) -> str: return reason[:REASON_MAX_LENGTH] +def _closing_step_kind(step: Step) -> str: + """`_run_closing`'s defensive fallback: classify a step for a synthetic + `ConductorFailure` when `_dispatch` raised instead of returning one.""" + if isinstance(step, SetpointStep): + return _STEP_KIND_SETPOINT + if isinstance(step, ActionStep): + return _STEP_KIND_ACTION + if isinstance(step, CaptureStep): + return _STEP_KIND_CAPTURE + if isinstance(step, ComputeStep): + return _STEP_KIND_COMPUTE + return _STEP_KIND_CHECK + + +def _closing_step_target(step: Step) -> str: + """Sibling of `_closing_step_kind`: the failure's `target` field.""" + if isinstance(step, ActionStep): + return step.name + if isinstance(step, ComputeStep): + return " ".join(step.command) + return step.address + + def _measurement_to_dict(reading: Measurement) -> dict[str, Any]: """JSON-clean projection of `Measurement` for the step payload. diff --git a/apps/api/src/cora/operation/errors.py b/apps/api/src/cora/operation/errors.py index d62bdeb6c5d..8c03a50a52e 100644 --- a/apps/api/src/cora/operation/errors.py +++ b/apps/api/src/cora/operation/errors.py @@ -13,6 +13,8 @@ (documented in CONTRIBUTING.md "BC-application-layer errors"). """ +from uuid import UUID + class UnauthorizedError(Exception): """The Authorize port denied the command.""" @@ -151,6 +153,59 @@ def __init__(self, reason: str) -> None: self.reason = reason +class ClosingCaptureBeforeBoundaryError(Exception): + """A closing step's `CaptureRef` names a capture only the pre-boundary + main steps declare. + + `conduct_from` starts the per-conduct `captures` dict EMPTY: only the + main steps from `boundary` onward re-run and can deposit into it (the + same "fails loud rather than resolving against stale data" contract + `execute_from` already holds for a main-step `CaptureRef`). Closing + steps always run in full regardless of `boundary`, so a closing + `CaptureRef` whose only declaring `CaptureStep` / capturing + `ComputeStep` sits before `boundary` would resolve against nothing + during THIS resume and fail as `UnresolvedCaptureRef` -- but inside + `_run_closing`'s per-step isolation, that failure is recorded and the + walk continues, silently converting a should-be-loud gap into a + recorded-and-continue one. Checked up front instead, so the operator + sees a 422 naming the missing capture and can pick a boundary at or + before the declaring step, rather than a closing_failures entry after + the fact. + """ + + def __init__(self, capture_name: str, boundary: int) -> None: + super().__init__( + f"closing step references capture {capture_name!r}, which only a " + f"pre-boundary main step (boundary={boundary}) declares; resume " + "starts captures empty, so this closing step would never see it" + ) + self.capture_name = capture_name + self.boundary = boundary + + +class UnsupportedClosingStepsError(Exception): + """A loop-driving conduct slice refuses a closing-bearing Recipe (v1 scope). + + `conduct_until_converged` / `conduct_until_advised` / `conduct_until_advised_from` + each re-walk ONE pass block repeatedly (loop-top abort, per-iteration + re-expansion); `_run_closing` runs once, on a real conduct terminal, and + has no defined place in a loop that may never terminate the way `conduct` + /`conduct_or_hold` do. Rather than silently drop the Recipe's closing + steps or guess when to run them, these three slices refuse the request + up front: well-formed, but this Recipe cannot be driven by a loop slice + until closing-in-a-loop is designed. Mapped to HTTP 422 (operator- + correctable: use `conduct` / `conduct_or_hold` for this Recipe, or + author a closing-less variant for loop-driven conduct). + """ + + def __init__(self, procedure_id: UUID) -> None: + super().__init__( + f"procedure {procedure_id} is bound to a Recipe with closing_steps, " + "which loop-driving conduct slices do not support" + ) + self.procedure_id = procedure_id + + class CheckFailedError(Exception): """A `CheckStep` either read a non-Good quality or its criterion did not match. diff --git a/apps/api/src/cora/operation/features/conduct_from_procedure/command.py b/apps/api/src/cora/operation/features/conduct_from_procedure/command.py index 841847cd52a..c821723a4ed 100644 --- a/apps/api/src/cora/operation/features/conduct_from_procedure/command.py +++ b/apps/api/src/cora/operation/features/conduct_from_procedure/command.py @@ -57,3 +57,11 @@ class ConductFromProcedureResult: artifacts: tuple[ArtifactRef, ...] = () outputs: Mapping[str, ArtifactRef] = field(default_factory=dict[str, ArtifactRef]) substrate_writes: Mapping[str, WriteValue] = field(default_factory=dict[str, WriteValue]) + closing_failures: tuple[ConductorFailure, ...] = () + """Every closing step that failed, threaded from `ConductorResult.closing_failures`. + + Always empty on `acquisition_halt=True`: closing runs only on a real + terminal (complete / abort), never on the halt-for-operator branch that + leaves the Procedure Running. Isolated from `failure`: never flips + `succeeded`. + """ diff --git a/apps/api/src/cora/operation/features/conduct_from_procedure/handler.py b/apps/api/src/cora/operation/features/conduct_from_procedure/handler.py index e2693b5effd..b5269d65300 100644 --- a/apps/api/src/cora/operation/features/conduct_from_procedure/handler.py +++ b/apps/api/src/cora/operation/features/conduct_from_procedure/handler.py @@ -64,18 +64,48 @@ ResolvedStepsRecordNotFoundError, load_procedure_with_events, ) -from cora.operation.conductor import Conductor, is_acquisition_halt, steps_from_payload -from cora.operation.errors import UnauthorizedError +from cora.operation.conductor import ( + CaptureStep, + ComputeStep, + Conductor, + SetpointStep, + Step, + is_acquisition_halt, + steps_from_payload, +) +from cora.operation.errors import ClosingCaptureBeforeBoundaryError, UnauthorizedError from cora.operation.features.conduct_from_procedure.command import ( ConductFromProcedure, ConductFromProcedureResult, ) +from cora.recipe.aggregates.recipe.body import CaptureRef _COMMAND_NAME = "ConductFromProcedure" _log = get_logger(__name__) +def _capture_names_declared_from(steps: tuple[Step, ...], boundary: int) -> frozenset[str]: + """Capture names a main step at or after `boundary` deposits. + + Only these are guaranteed to be populated during THIS resume: `captures` + starts empty and the tail from `boundary` is all that re-runs.""" + declared: set[str] = set() + for step in steps[boundary:]: + if isinstance(step, (CaptureStep, ComputeStep)) and step.capture_name is not None: + declared.add(step.capture_name) + return frozenset(declared) + + +def _closing_capture_refs(closing_steps: tuple[Step, ...]) -> frozenset[str]: + """Capture names a closing step's `SetpointStep.value` reads via `CaptureRef`.""" + return frozenset( + step.value.capture_name + for step in closing_steps + if isinstance(step, SetpointStep) and isinstance(step.value, CaptureRef) + ) + + class Handler(Protocol): """Callable interface every conduct_from_procedure handler implements.""" @@ -157,6 +187,7 @@ async def handler( if record is None: raise ResolvedStepsRecordNotFoundError(command.procedure_id) steps = steps_from_payload(record.payload["resolved_steps"]) + closing_steps = steps_from_payload(record.payload.get("resolved_closing_steps", ())) # Upper-bound guard: a boundary PAST the pinned step count would replay # an empty tail and silently auto-complete with nothing re-driven. The @@ -167,11 +198,25 @@ async def handler( if command.re_establishment_boundary > len(steps): raise InvalidProcedureReEstablishmentBoundaryError(command.re_establishment_boundary) + # A closing CaptureRef must resolve against THIS resume's captures, + # which start empty and fill only from `boundary` onward -- a name + # only a pre-boundary main step declares would otherwise fail deep + # inside _run_closing's per-step isolation instead of up front. + # See ClosingCaptureBeforeBoundaryError. + missing_captures = _closing_capture_refs(closing_steps) - _capture_names_declared_from( + steps, command.re_establishment_boundary + ) + if missing_captures: + raise ClosingCaptureBeforeBoundaryError( + sorted(missing_captures)[0], command.re_establishment_boundary + ) + result = await conductor.conduct_from( procedure_id=command.procedure_id, principal_id=principal_id, correlation_id=correlation_id, steps=steps, + closing_steps=closing_steps, boundary=command.re_establishment_boundary, # The pre-hold conduct's observed kind (folded onto the Held # Procedure) so the terminal event reflects the FULL provenance, @@ -207,6 +252,7 @@ async def handler( artifacts=result.artifacts, outputs=result.outputs, substrate_writes=result.substrate_writes, + closing_failures=result.closing_failures, ) return handler diff --git a/apps/api/src/cora/operation/features/conduct_from_procedure/route.py b/apps/api/src/cora/operation/features/conduct_from_procedure/route.py index ac662429ae7..a9ec1605935 100644 --- a/apps/api/src/cora/operation/features/conduct_from_procedure/route.py +++ b/apps/api/src/cora/operation/features/conduct_from_procedure/route.py @@ -28,6 +28,7 @@ ) from cora.operation._conduct_wire import ( ConductorFailureResponse, + closing_failures_to_wire, failure_to_wire, substrate_writes_to_wire, ) @@ -79,6 +80,13 @@ class ConductFromProcedureResponse(BaseModel): "setpoints re-driven and nothing having put them back." ), ) + closing_failures: list[ConductorFailureResponse] = Field( + default_factory=list[ConductorFailureResponse], + description=( + "Every closing step that failed. Always empty on " + "`acquisition_halt=True`. Never flips `succeeded`." + ), + ) def result_to_wire(result: ConductFromProcedureResult) -> ConductFromProcedureResponse: @@ -95,6 +103,7 @@ def result_to_wire(result: ConductFromProcedureResult) -> ConductFromProcedureRe failure=failure_to_wire(result.failure) if result.failure is not None else None, actuation_kind=result.actuation_kind, substrate_writes=substrate_writes_to_wire(result.substrate_writes), + closing_failures=closing_failures_to_wire(result.closing_failures), ) @@ -131,7 +140,11 @@ def _get_handler(request: Request) -> Handler: ), }, status.HTTP_422_UNPROCESSABLE_CONTENT: { - "description": "Path parameter or request body failed schema validation.", + "description": ( + "Path parameter or request body failed schema validation, OR " + "a closing step's CaptureRef names a capture only a " + "pre-boundary main step declares (ClosingCaptureBeforeBoundaryError)." + ), }, status.HTTP_500_INTERNAL_SERVER_ERROR: { "model": ErrorResponse, diff --git a/apps/api/src/cora/operation/features/conduct_from_procedure/tool.py b/apps/api/src/cora/operation/features/conduct_from_procedure/tool.py index 4623b8ec25a..42e7117b1c1 100644 --- a/apps/api/src/cora/operation/features/conduct_from_procedure/tool.py +++ b/apps/api/src/cora/operation/features/conduct_from_procedure/tool.py @@ -42,6 +42,13 @@ class _ToolResult(BaseModel): "acquisition halt too." ), ) + closing_failures: list[dict[str, Any]] = Field( + default_factory=list[dict[str, Any]], + description=( + "Every closing step that failed. Always empty on an acquisition " + "halt. Never flips `succeeded`." + ), + ) def register(mcp: FastMCP, *, get_handler: Callable[[], Handler]) -> None: @@ -97,4 +104,5 @@ async def conduct_from_procedure_tool( # pyright: ignore[reportUnusedFunction] failure=wire.failure.model_dump() if wire.failure is not None else None, actuation_kind=wire.actuation_kind, substrate_writes=wire.substrate_writes, + closing_failures=[f.model_dump() for f in wire.closing_failures], ) diff --git a/apps/api/src/cora/operation/features/conduct_or_hold_procedure/command.py b/apps/api/src/cora/operation/features/conduct_or_hold_procedure/command.py index 232af681cb1..e1624ebec26 100644 --- a/apps/api/src/cora/operation/features/conduct_or_hold_procedure/command.py +++ b/apps/api/src/cora/operation/features/conduct_or_hold_procedure/command.py @@ -57,3 +57,10 @@ class ConductOrHoldProcedureResult: artifacts: tuple[ArtifactRef, ...] = () outputs: Mapping[str, ArtifactRef] = field(default_factory=dict[str, ArtifactRef]) substrate_writes: Mapping[str, WriteValue] = field(default_factory=dict[str, WriteValue]) + closing_failures: tuple[ConductorFailure, ...] = () + """Every closing step that failed, threaded from `ConductorResult.closing_failures`. + + Always empty on a `held=True` outcome: closing runs only on a real + terminal (Completed / Aborted), never on Held. Isolated from `failure`: + a closing failure never flips `succeeded`. + """ diff --git a/apps/api/src/cora/operation/features/conduct_or_hold_procedure/handler.py b/apps/api/src/cora/operation/features/conduct_or_hold_procedure/handler.py index eeceb1a166d..7839f492be6 100644 --- a/apps/api/src/cora/operation/features/conduct_or_hold_procedure/handler.py +++ b/apps/api/src/cora/operation/features/conduct_or_hold_procedure/handler.py @@ -121,7 +121,7 @@ async def handler( if procedure is None: raise ProcedureNotFoundError(command.procedure_id) - steps = await resolve_and_pin_conduct_steps( + steps, closing_steps = await resolve_and_pin_conduct_steps( deps, command_name=_COMMAND_NAME, procedure=procedure, @@ -140,6 +140,7 @@ async def handler( causation_id=causation_id, surface_id=surface_id, steps=steps, + closing_steps=closing_steps, ) _log.info( @@ -165,6 +166,7 @@ async def handler( artifacts=result.artifacts, outputs=result.outputs, substrate_writes=result.substrate_writes, + closing_failures=result.closing_failures, ) return handler diff --git a/apps/api/src/cora/operation/features/conduct_or_hold_procedure/route.py b/apps/api/src/cora/operation/features/conduct_or_hold_procedure/route.py index e8946b12430..afad9856c50 100644 --- a/apps/api/src/cora/operation/features/conduct_or_hold_procedure/route.py +++ b/apps/api/src/cora/operation/features/conduct_or_hold_procedure/route.py @@ -37,6 +37,7 @@ STEP_BATCH_MAX, ConductorFailureResponse, StepRequest, + closing_failures_to_wire, failure_to_wire, step_from_wire, substrate_writes_to_wire, @@ -92,6 +93,14 @@ class ConductOrHoldProcedureResponse(BaseModel): "have not run, so this is what was left set." ), ) + closing_failures: list[ConductorFailureResponse] = Field( + default_factory=list[ConductorFailureResponse], + description=( + "Every closing step that failed. Always empty on `held=True`: " + "closing runs only on a real terminal, never on Held. Never " + "flips `succeeded`." + ), + ) def result_to_wire(result: ConductOrHoldProcedureResult) -> ConductOrHoldProcedureResponse: @@ -107,6 +116,7 @@ def result_to_wire(result: ConductOrHoldProcedureResult) -> ConductOrHoldProcedu failure=failure_to_wire(result.failure) if result.failure is not None else None, actuation_kind=result.actuation_kind, substrate_writes=substrate_writes_to_wire(result.substrate_writes), + closing_failures=closing_failures_to_wire(result.closing_failures), ) diff --git a/apps/api/src/cora/operation/features/conduct_or_hold_procedure/tool.py b/apps/api/src/cora/operation/features/conduct_or_hold_procedure/tool.py index 615dcdbffc2..9c13a8db3fd 100644 --- a/apps/api/src/cora/operation/features/conduct_or_hold_procedure/tool.py +++ b/apps/api/src/cora/operation/features/conduct_or_hold_procedure/tool.py @@ -44,6 +44,12 @@ class _ToolResult(BaseModel): "outcome too: a paused Procedure's closing steps have not run." ), ) + closing_failures: list[dict[str, Any]] = Field( + default_factory=list[dict[str, Any]], + description=( + "Every closing step that failed. Always empty on `held=True`. Never flips `succeeded`." + ), + ) def register(mcp: FastMCP, *, get_handler: Callable[[], Handler]) -> None: @@ -92,4 +98,5 @@ async def conduct_or_hold_procedure_tool( # pyright: ignore[reportUnusedFunctio failure=wire.failure.model_dump() if wire.failure is not None else None, actuation_kind=wire.actuation_kind, substrate_writes=wire.substrate_writes, + closing_failures=[f.model_dump() for f in wire.closing_failures], ) diff --git a/apps/api/src/cora/operation/features/conduct_procedure/command.py b/apps/api/src/cora/operation/features/conduct_procedure/command.py index a41859f17d5..f1a4c4c47d5 100644 --- a/apps/api/src/cora/operation/features/conduct_procedure/command.py +++ b/apps/api/src/cora/operation/features/conduct_procedure/command.py @@ -75,10 +75,15 @@ class ConductProcedureResult: substrate_writes: Mapping[str, WriteValue] = field(default_factory=dict[str, WriteValue]) """Every address the conduct wrote, in first-write order, last value. - Threaded from `ConductorResult.substrate_writes`. On a HALT this is - what CORA left set: the step loop returns at the failing step, so a - recipe's own closing steps never run and nothing restores what - earlier steps changed. Surfacing it on the response means an - operator reads it in the same breath as the failure, instead of - reconstructing it from the step journal. + Threaded from `ConductorResult.substrate_writes`. Includes the Recipe's + closing steps, which run on a real terminal (Completed or Aborted) and + merge their own writes into this ledger. + """ + closing_failures: tuple[ConductorFailure, ...] = () + """Every closing step that failed, threaded from `ConductorResult.closing_failures`. + + Isolated from `failure`: a closing failure never flips `succeeded`, and + one closing step failing does not stop the rest of the closing walk. + Empty when the bound Recipe has no closing steps, or every closing step + ran clean. """ diff --git a/apps/api/src/cora/operation/features/conduct_procedure/handler.py b/apps/api/src/cora/operation/features/conduct_procedure/handler.py index e8aebd2d0ae..6ce0cf440d6 100644 --- a/apps/api/src/cora/operation/features/conduct_procedure/handler.py +++ b/apps/api/src/cora/operation/features/conduct_procedure/handler.py @@ -139,7 +139,7 @@ async def handler( if procedure is None: raise ProcedureNotFoundError(command.procedure_id) - steps = await resolve_and_pin_conduct_steps( + steps, closing_steps = await resolve_and_pin_conduct_steps( deps, command_name=_COMMAND_NAME, procedure=procedure, @@ -158,6 +158,7 @@ async def handler( causation_id=causation_id, surface_id=surface_id, steps=steps, + closing_steps=closing_steps, ) _log.info( @@ -181,6 +182,7 @@ async def handler( artifacts=result.artifacts, outputs=result.outputs, substrate_writes=result.substrate_writes, + closing_failures=result.closing_failures, ) return handler diff --git a/apps/api/src/cora/operation/features/conduct_procedure/route.py b/apps/api/src/cora/operation/features/conduct_procedure/route.py index 865acc5e59e..08e6ddaf946 100644 --- a/apps/api/src/cora/operation/features/conduct_procedure/route.py +++ b/apps/api/src/cora/operation/features/conduct_procedure/route.py @@ -45,6 +45,7 @@ STEP_BATCH_MAX, ConductorFailureResponse, StepRequest, + closing_failures_to_wire, failure_to_wire, step_from_wire, substrate_writes_to_wire, @@ -94,12 +95,20 @@ class ConductProcedureResponse(BaseModel): default_factory=dict[str, int | float | bool | str | list[Any]], description=( "Every control address this conduct wrote, in first-write " - "order, carrying the last value written to each. CORA does " - "not restore what it sets, and a halt returns at the failing " - "step without running the recipe's remaining steps, so on a " - "failed conduct this is what was left set with nothing having " - "put it back. Reports what was WRITTEN, not what changed: a " - "write whose value already matched the address still appears." + "order, carrying the last value written to each. Includes the " + "recipe's closing steps, which run on a real terminal " + "(Completed or Aborted). Reports what was WRITTEN, not what " + "changed: a write whose value already matched the address " + "still appears." + ), + ) + closing_failures: list[ConductorFailureResponse] = Field( + default_factory=list[ConductorFailureResponse], + description=( + "Every closing step that failed. Isolated from `failure`: a " + "closing failure never flips `succeeded`, and one closing " + "step failing does not stop the rest of the closing walk. " + "Empty when the recipe has no closing steps, or all ran clean." ), ) @@ -116,6 +125,7 @@ def result_to_wire(result: ConductProcedureResult) -> ConductProcedureResponse: failure=failure_to_wire(result.failure) if result.failure is not None else None, actuation_kind=result.actuation_kind, substrate_writes=substrate_writes_to_wire(result.substrate_writes), + closing_failures=closing_failures_to_wire(result.closing_failures), ) diff --git a/apps/api/src/cora/operation/features/conduct_procedure/tool.py b/apps/api/src/cora/operation/features/conduct_procedure/tool.py index 8a57a5ccd18..5a71b291541 100644 --- a/apps/api/src/cora/operation/features/conduct_procedure/tool.py +++ b/apps/api/src/cora/operation/features/conduct_procedure/tool.py @@ -43,6 +43,13 @@ class _ToolResult(BaseModel): "it back." ), ) + closing_failures: list[dict[str, Any]] = Field( + default_factory=list[dict[str, Any]], + description=( + "Every closing step that failed. Never flips `succeeded`; empty " + "when the recipe has no closing steps, or all ran clean." + ), + ) def register(mcp: FastMCP, *, get_handler: Callable[[], Handler]) -> None: @@ -89,4 +96,5 @@ async def conduct_procedure_tool( # pyright: ignore[reportUnusedFunction] failure=wire.failure.model_dump() if wire.failure is not None else None, actuation_kind=wire.actuation_kind, substrate_writes=wire.substrate_writes, + closing_failures=[f.model_dump() for f in wire.closing_failures], ) diff --git a/apps/api/src/cora/operation/features/conduct_until_advised/handler.py b/apps/api/src/cora/operation/features/conduct_until_advised/handler.py index ee8f75c9b72..9d40e552979 100644 --- a/apps/api/src/cora/operation/features/conduct_until_advised/handler.py +++ b/apps/api/src/cora/operation/features/conduct_until_advised/handler.py @@ -42,7 +42,11 @@ load_procedure_with_events, ) from cora.operation.conductor import Conductor -from cora.operation.errors import SteeringWireMismatchError, UnauthorizedError +from cora.operation.errors import ( + SteeringWireMismatchError, + UnauthorizedError, + UnsupportedClosingStepsError, +) from cora.operation.features.conduct_until_advised.command import ( ConductUntilAdvised, ConductUntilAdvisedResult, @@ -134,7 +138,7 @@ async def handler( if procedure is None: raise ProcedureNotFoundError(command.procedure_id) - steps = await resolve_and_pin_conduct_steps( + steps, closing_steps = await resolve_and_pin_conduct_steps( deps, command_name=_COMMAND_NAME, procedure=procedure, @@ -145,6 +149,12 @@ async def handler( correlation_id=correlation_id, causation_id=causation_id, ) + # v1 scope: a loop that re-walks one pass block repeatedly has no + # defined place to run _run_closing. Refuse outright rather than + # silently drop the Recipe's closing steps or guess when to run + # them. See [[project_conduct_closing_steps_design]]. + if closing_steps: + raise UnsupportedClosingStepsError(command.procedure_id) llm_calls: list[SteeringLlmCall] = [] decide_port = build_decide_port( diff --git a/apps/api/src/cora/operation/features/conduct_until_advised_from/handler.py b/apps/api/src/cora/operation/features/conduct_until_advised_from/handler.py index 20733b13f4b..c293af28fa8 100644 --- a/apps/api/src/cora/operation/features/conduct_until_advised_from/handler.py +++ b/apps/api/src/cora/operation/features/conduct_until_advised_from/handler.py @@ -66,6 +66,7 @@ from cora.operation.errors import ( SteeringWireMismatchError, UnauthorizedError, + UnsupportedClosingStepsError, ) from cora.operation.features.conduct_until_advised_from.command import ( ConductUntilAdvisedFrom, @@ -175,6 +176,13 @@ async def handler( if record is None: raise ResolvedStepsRecordNotFoundError(command.procedure_id) steps = steps_from_payload(record.payload["resolved_steps"]) + # v1 scope, mirroring conduct_until_advised's forward-direction refusal: + # the original pinned record already carries resolved_closing_steps + # when the bound Recipe has any (conduct_until_advised's own + # resolve_and_pin_conduct_steps call pinned it), so no fresh Recipe + # load is needed here. See [[project_conduct_closing_steps_design]]. + if record.payload.get("resolved_closing_steps"): + raise UnsupportedClosingStepsError(command.procedure_id) # RECONSTRUCT the brain's history from the self-describing outcome rows # (each carries its own point + measurements), so this is a sort-then-map diff --git a/apps/api/src/cora/operation/features/conduct_until_converged/handler.py b/apps/api/src/cora/operation/features/conduct_until_converged/handler.py index c788a40b207..a8bb8005b04 100644 --- a/apps/api/src/cora/operation/features/conduct_until_converged/handler.py +++ b/apps/api/src/cora/operation/features/conduct_until_converged/handler.py @@ -44,7 +44,7 @@ load_procedure_with_events, ) from cora.operation.conductor import Conductor -from cora.operation.errors import UnauthorizedError +from cora.operation.errors import UnauthorizedError, UnsupportedClosingStepsError from cora.operation.features.conduct_until_converged.command import ( ConductUntilConverged, ConductUntilConvergedResult, @@ -127,7 +127,7 @@ async def handler( if procedure is None: raise ProcedureNotFoundError(command.procedure_id) - steps = await resolve_and_pin_conduct_steps( + steps, closing_steps = await resolve_and_pin_conduct_steps( deps, command_name=_COMMAND_NAME, procedure=procedure, @@ -138,6 +138,13 @@ async def handler( correlation_id=correlation_id, causation_id=causation_id, ) + # v1 scope: a loop that re-walks one pass block repeatedly has no + # defined place to run _run_closing (it runs once, on a real conduct + # terminal). Refuse outright rather than silently drop the Recipe's + # closing steps or guess when to run them. See + # [[project_conduct_closing_steps_design]]. + if closing_steps: + raise UnsupportedClosingStepsError(command.procedure_id) # The command's explicit cap overrides the registered one only when # supplied; otherwise the loop honors the Procedure's declared diff --git a/apps/api/src/cora/operation/features/register_procedure_from_recipe/decider.py b/apps/api/src/cora/operation/features/register_procedure_from_recipe/decider.py index f2b6ee21003..b22adfd91a7 100644 --- a/apps/api/src/cora/operation/features/register_procedure_from_recipe/decider.py +++ b/apps/api/src/cora/operation/features/register_procedure_from_recipe/decider.py @@ -50,7 +50,7 @@ from typing import Any from uuid import UUID -from cora.operation._recipe_expansion import canonical_json_bytes, steps_to_wire +from cora.operation._recipe_expansion import canonical_json_bytes, steps_to_wire_with_closing from cora.operation.aggregates.procedure import ( PROCEDURE_KIND_MAX_LENGTH, RECIPE_EXPANSION_STEP_MAX, @@ -78,15 +78,19 @@ from cora.shared.json_schema_validation import validate_values_against_schema -def _hash_steps(steps: tuple[Step, ...]) -> str: +def _hash_steps(steps: tuple[Step, ...], closing_steps: tuple[Step, ...]) -> str: """Content-address the expanded Step tuple per memo §RecipeExpansionRecorded. Hashing the expanded steps (not the unexpanded Recipe template) pins what the Conductor will actually execute, so a Recipe re-version that produces equivalent expanded steps still hashes - identically. + identically. One combined pin over main + closing (no aliasing): + an empty `closing_steps` reproduces the pre-closing-steps hash + byte-for-byte, so no existing pinned expansion is invalidated. """ - return hashlib.sha256(canonical_json_bytes(steps_to_wire(steps))).hexdigest() + return hashlib.sha256( + canonical_json_bytes(steps_to_wire_with_closing(steps, closing_steps)) + ).hexdigest() def _hash_bindings(bindings: Mapping[str, Any]) -> str: @@ -136,16 +140,17 @@ def decide( raise InvalidProcedureIterationCapError(cap) steps_first = expansion_port.expand(recipe.steps, bindings_dict) - if len(steps_first) > RECIPE_EXPANSION_STEP_MAX: - raise RecipeExpansionOverflowError( - step_count=len(steps_first), cap=RECIPE_EXPANSION_STEP_MAX - ) + closing_first = expansion_port.expand(recipe.closing_steps, bindings_dict) + total_count = len(steps_first) + len(closing_first) + if total_count > RECIPE_EXPANSION_STEP_MAX: + raise RecipeExpansionOverflowError(step_count=total_count, cap=RECIPE_EXPANSION_STEP_MAX) - # Determinism check: re-expand and compare. The port wraps a pure - # function; any divergence is a server-side bug in the port or the + # Determinism check: re-expand both lists and compare. The port wraps a + # pure function; any divergence is a server-side bug in the port or the # recipe body. steps_second = expansion_port.expand(recipe.steps, bindings_dict) - if steps_first != steps_second: + closing_second = expansion_port.expand(recipe.closing_steps, bindings_dict) + if steps_first != steps_second or closing_first != closing_second: raise RecipeExpansionDeterminismError(recipe.id) return [ @@ -169,9 +174,9 @@ def decide( capability_version=capability.version, bindings=bindings_dict, expansion_port_version=expansion_port.version, - steps_hash=_hash_steps(steps_first), + steps_hash=_hash_steps(steps_first, closing_first), bindings_hash=_hash_bindings(bindings_dict), - step_count=len(steps_first), + step_count=total_count, occurred_at=now, ), ] diff --git a/apps/api/src/cora/operation/routes.py b/apps/api/src/cora/operation/routes.py index 6073d3feb18..1d9dbc00ce3 100644 --- a/apps/api/src/cora/operation/routes.py +++ b/apps/api/src/cora/operation/routes.py @@ -78,6 +78,7 @@ ) from cora.operation.errors import ( AssetNotPseudoAxisError, + ClosingCaptureBeforeBoundaryError, PartitionRuleNotFoundError, PseudoAxisCommandOutsideRangeError, PseudoAxisConstituentDispatchError, @@ -87,6 +88,7 @@ PseudoAxisSingularityExceededError, SteeringWireMismatchError, UnauthorizedError, + UnsupportedClosingStepsError, ) from cora.operation.features import ( abort_procedure, @@ -365,6 +367,16 @@ def register_operation_routes(app: FastAPI) -> None: # setpoints (the Conductor's pre-FSM wire guard). Well-formed request, # unprocessable against the recipe; operator aligns the space + retries. SteeringWireMismatchError, + # conduct_from: a closing step's CaptureRef names a capture only a + # pre-boundary main step declares -- resume starts captures empty, so + # this resume would never populate it. Operator picks a boundary at + # or before the declaring step, or drops the closing CaptureRef. + ClosingCaptureBeforeBoundaryError, + # The three loop-driving conduct slices (conduct_until_converged, + # conduct_until_advised, conduct_until_advised_from) refuse a + # closing-bearing Recipe outright: _run_closing has no defined place + # in a loop that re-walks one pass block repeatedly (v1 scope). + UnsupportedClosingStepsError, ): app.add_exception_handler(unprocessable_cls, _handle_unprocessable) # Server-side determinism bugs / data corruption: HTTP 500. Distinct diff --git a/apps/api/src/cora/recipe/aggregates/recipe/events.py b/apps/api/src/cora/recipe/aggregates/recipe/events.py index 28d9719ea65..61c382a2955 100644 --- a/apps/api/src/cora/recipe/aggregates/recipe/events.py +++ b/apps/api/src/cora/recipe/aggregates/recipe/events.py @@ -63,6 +63,9 @@ class RecipeDefined: Status is implicit (`Defined`); the evolver sets it. All declarative fields are present in the genesis payload. + + `closing_steps` is additive and optional (default `()`); see + `Recipe.closing_steps`. """ recipe_id: UUID @@ -70,6 +73,7 @@ class RecipeDefined: capability_id: UUID steps: tuple[RecipeStep, ...] occurred_at: datetime + closing_steps: tuple[RecipeStep, ...] = () @dataclass(frozen=True) @@ -80,13 +84,14 @@ class RecipeVersioned: full step sequence REPLACES wholesale (a new version IS a new declaration). `name` and `capability_id` are PRESERVED across versions; re-binding to a different Capability requires a new - Recipe. + Recipe. `closing_steps` replaces wholesale too, alongside `steps`. """ recipe_id: UUID version_tag: str steps: tuple[RecipeStep, ...] occurred_at: datetime + closing_steps: tuple[RecipeStep, ...] = () @dataclass(frozen=True) @@ -129,6 +134,7 @@ def to_payload(event: RecipeEvent) -> dict[str, Any]: capability_id=capability_id, steps=steps, occurred_at=occurred_at, + closing_steps=closing_steps, ): return { "recipe_id": str(recipe_id), @@ -136,18 +142,21 @@ def to_payload(event: RecipeEvent) -> dict[str, Any]: "capability_id": str(capability_id), "steps": steps_to_dict(steps), "occurred_at": occurred_at.isoformat(), + "closing": steps_to_dict(closing_steps), } case RecipeVersioned( recipe_id=recipe_id, version_tag=version_tag, steps=steps, occurred_at=occurred_at, + closing_steps=closing_steps, ): return { "recipe_id": str(recipe_id), "version_tag": version_tag, "steps": steps_to_dict(steps), "occurred_at": occurred_at.isoformat(), + "closing": steps_to_dict(closing_steps), } case RecipeDeprecated( recipe_id=recipe_id, @@ -189,6 +198,9 @@ def from_stored(stored: StoredEvent) -> RecipeEvent: capability_id=UUID(payload["capability_id"]), steps=steps_from_dict(payload["steps"]), occurred_at=datetime.fromisoformat(payload["occurred_at"]), + # `.get`, not `["closing"]`: a stored event from before + # closing steps existed has no such key. + closing_steps=steps_from_dict(payload.get("closing", {"steps": []})), ), extra=_PAYLOAD_PARSE_EXTRA, ) @@ -200,6 +212,7 @@ def from_stored(stored: StoredEvent) -> RecipeEvent: version_tag=payload["version_tag"], steps=steps_from_dict(payload["steps"]), occurred_at=datetime.fromisoformat(payload["occurred_at"]), + closing_steps=steps_from_dict(payload.get("closing", {"steps": []})), ), extra=_PAYLOAD_PARSE_EXTRA, ) diff --git a/apps/api/src/cora/recipe/aggregates/recipe/evolver.py b/apps/api/src/cora/recipe/aggregates/recipe/evolver.py index 0351c472136..e215c916f5b 100644 --- a/apps/api/src/cora/recipe/aggregates/recipe/evolver.py +++ b/apps/api/src/cora/recipe/aggregates/recipe/evolver.py @@ -16,11 +16,12 @@ ## Replace vs preserve on each arm -- `RecipeVersioned` REPLACES `steps` with the new event's tuple (a - new version IS a new declaration). PRESERVES `name`, +- `RecipeVersioned` REPLACES `steps` AND `closing_steps` with the new + event's tuples (a new version IS a new declaration, and a version + can fix a broken closing step too). PRESERVES `name`, `capability_id`, and `replaced_by_recipe_id`. - `RecipeDeprecated` PRESERVES all declarative fields (steps, - capability_id, name, version) and ADDS the + closing_steps, capability_id, name, version) and ADDS the `replaced_by_recipe_id` pointer. Operators reading a deprecated Recipe still see what it declared (audit-critical). @@ -54,6 +55,7 @@ def evolve(state: Recipe | None, event: RecipeEvent) -> Recipe: name=name, capability_id=capability_id, steps=steps, + closing_steps=closing_steps, ): _ = state # genesis event; prior state ignored return Recipe( @@ -62,8 +64,9 @@ def evolve(state: Recipe | None, event: RecipeEvent) -> Recipe: capability_id=capability_id, steps=steps, status=RecipeStatus.DEFINED, + closing_steps=closing_steps, ) - case RecipeVersioned(version_tag=version_tag, steps=steps): + case RecipeVersioned(version_tag=version_tag, steps=steps, closing_steps=closing_steps): prior = require_state(state, "RecipeVersioned") return Recipe( id=prior.id, @@ -73,6 +76,7 @@ def evolve(state: Recipe | None, event: RecipeEvent) -> Recipe: status=RecipeStatus.VERSIONED, version=version_tag, replaced_by_recipe_id=prior.replaced_by_recipe_id, + closing_steps=closing_steps, ) case RecipeDeprecated(replaced_by_recipe_id=replaced_by_recipe_id): prior = require_state(state, "RecipeDeprecated") @@ -84,6 +88,7 @@ def evolve(state: Recipe | None, event: RecipeEvent) -> Recipe: status=RecipeStatus.DEPRECATED, version=prior.version, replaced_by_recipe_id=replaced_by_recipe_id, + closing_steps=prior.closing_steps, ) case _: # pragma: no cover # exhaustiveness guard assert_never(event) diff --git a/apps/api/src/cora/recipe/aggregates/recipe/state.py b/apps/api/src/cora/recipe/aggregates/recipe/state.py index 7621ebdf505..58d75f75cb1 100644 --- a/apps/api/src/cora/recipe/aggregates/recipe/state.py +++ b/apps/api/src/cora/recipe/aggregates/recipe/state.py @@ -233,6 +233,14 @@ class Recipe: one is deprecated with replacement. None on Deprecated-without-replacement and on Defined/Versioned. LOINC `MAP_TO` precedent matching `Capability.replaced_by_capability_id`. + + `closing_steps` is an additive, OPTIONAL second step list (default + `()`, no non-emptiness check): steps the Conductor walks after + `steps` ends on a real terminal (Completed or Aborted), never on a + Held pause. Authored, never derived; unlike `steps`, an empty + `closing_steps` is the common case, not an error. Replaced wholesale + by `version_recipe` alongside `steps`. See + [[project_conduct_closing_steps_design]]. """ id: UUID @@ -242,6 +250,7 @@ class Recipe: status: RecipeStatus = RecipeStatus.DEFINED version: str | None = None replaced_by_recipe_id: UUID | None = None + closing_steps: tuple[RecipeStep, ...] = () def __post_init__(self) -> None: if not self.steps: diff --git a/apps/api/src/cora/recipe/features/define_recipe/command.py b/apps/api/src/cora/recipe/features/define_recipe/command.py index 27b9bbeae35..33feabb8409 100644 --- a/apps/api/src/cora/recipe/features/define_recipe/command.py +++ b/apps/api/src/cora/recipe/features/define_recipe/command.py @@ -27,8 +27,13 @@ @dataclass(frozen=True) class DefineRecipe: - """Define a new Recipe against an existing Capability.""" + """Define a new Recipe against an existing Capability. + + `closing_steps` is additive and optional (default `()`); see + `Recipe.closing_steps`. + """ name: str capability_id: UUID steps: tuple[RecipeStep, ...] + closing_steps: tuple[RecipeStep, ...] = () diff --git a/apps/api/src/cora/recipe/features/define_recipe/decider.py b/apps/api/src/cora/recipe/features/define_recipe/decider.py index 0c50d7fa52c..3a0c012b278 100644 --- a/apps/api/src/cora/recipe/features/define_recipe/decider.py +++ b/apps/api/src/cora/recipe/features/define_recipe/decider.py @@ -50,6 +50,7 @@ def decide( name=name, capability_id=command.capability_id, steps=command.steps, + closing_steps=command.closing_steps, ) return [ RecipeDefined( @@ -58,5 +59,6 @@ def decide( capability_id=command.capability_id, steps=command.steps, occurred_at=now, + closing_steps=command.closing_steps, ) ] diff --git a/apps/api/src/cora/recipe/features/define_recipe/handler.py b/apps/api/src/cora/recipe/features/define_recipe/handler.py index 5369c5b0dee..c1634a7ec42 100644 --- a/apps/api/src/cora/recipe/features/define_recipe/handler.py +++ b/apps/api/src/cora/recipe/features/define_recipe/handler.py @@ -110,9 +110,15 @@ async def handler( capability = await load_capability(deps.event_store, command.capability_id) if capability is None: raise CapabilityNotFoundError(command.capability_id) - validate_recipe_steps_against_capability_schema(command.steps, capability.parameters_schema) - validate_capture_refs(command.steps) - validate_output_refs(command.steps) + # ONE concatenated walk, not two: `validate_output_refs`'s one-sink + # rule asserts at end-of-call, so validating closing_steps as a + # second pass starting cold would falsely reject a valid + # chained-compute recipe. This also gives "closing may read a main + # capture, not the reverse" for free, with no new concept. + all_steps = command.steps + command.closing_steps + validate_recipe_steps_against_capability_schema(all_steps, capability.parameters_schema) + validate_capture_refs(all_steps) + validate_output_refs(all_steps) new_id = deps.id_generator.new_id() now = deps.clock.now() diff --git a/apps/api/src/cora/recipe/features/define_recipe/route.py b/apps/api/src/cora/recipe/features/define_recipe/route.py index 730a8b4073b..7a76ddf20ce 100644 --- a/apps/api/src/cora/recipe/features/define_recipe/route.py +++ b/apps/api/src/cora/recipe/features/define_recipe/route.py @@ -44,6 +44,15 @@ class DefineRecipeRequest(BaseModel): "`{__binding__: name}` to reference a Capability parameter." ), ) + closing_steps: dict[str, Any] = Field( + default_factory=lambda: {"steps": []}, + description=( + "Optional wire-format closing-step sequence, same shape as " + "`steps`: `{steps: [...]}`. The Conductor walks these after " + "`steps` ends on a real terminal (Completed or Aborted), never " + "on a Held pause. Empty by default; most recipes have none." + ), + ) class DefineRecipeResponse(BaseModel): @@ -111,6 +120,7 @@ async def post_recipes( name=body.name, capability_id=body.capability_id, steps=steps_from_dict(body.steps), + closing_steps=steps_from_dict(body.closing_steps), ), principal_id=principal_id, correlation_id=cid, diff --git a/apps/api/src/cora/recipe/features/define_recipe/tool.py b/apps/api/src/cora/recipe/features/define_recipe/tool.py index e4e17138fe7..6d12ff6844b 100644 --- a/apps/api/src/cora/recipe/features/define_recipe/tool.py +++ b/apps/api/src/cora/recipe/features/define_recipe/tool.py @@ -64,6 +64,18 @@ async def define_recipe_tool( # pyright: ignore[reportUnusedFunction] ), ), ], + closing_steps: Annotated[ + dict[str, Any] | None, + Field( + default=None, + description=( + "Optional closing-step sequence, same shape as `steps`. " + "The Conductor walks these after `steps` ends on a real " + "terminal (Completed or Aborted), never on a Held pause. " + "Omit or pass None for no closing steps (the common case)." + ), + ), + ] = None, ) -> DefineRecipeOutput: handler = get_handler() recipe_id = await handler( @@ -71,6 +83,9 @@ async def define_recipe_tool( # pyright: ignore[reportUnusedFunction] name=name, capability_id=capability_id, steps=steps_from_dict(steps), + closing_steps=steps_from_dict( + closing_steps if closing_steps is not None else {"steps": []} + ), ), principal_id=get_mcp_principal_id(ctx), correlation_id=current_correlation_id(), diff --git a/apps/api/src/cora/recipe/features/get_recipe/route.py b/apps/api/src/cora/recipe/features/get_recipe/route.py index 0b7dbb9f6ab..a9337df948d 100644 --- a/apps/api/src/cora/recipe/features/get_recipe/route.py +++ b/apps/api/src/cora/recipe/features/get_recipe/route.py @@ -46,7 +46,9 @@ class RecipeResponse(BaseModel): Deprecated-without-replacement; populated when a deprecation supplied a successor pointer. `created_at` / `versioned_at` / `deprecated_at` are projection-sourced lifecycle timestamps - (Path C); see module docstring for null semantics. + (Path C); see module docstring for null semantics. `closing_steps` + mirrors `steps`' wire shape; empty for the common case of a recipe + with no closing steps. """ id: UUID @@ -55,6 +57,7 @@ class RecipeResponse(BaseModel): status: str version: str | None steps: dict[str, Any] + closing_steps: dict[str, Any] replaced_by_recipe_id: UUID | None created_at: datetime | None = None versioned_at: datetime | None = None @@ -111,6 +114,7 @@ async def get_recipes( status=recipe.status.value, version=recipe.version, steps=steps_to_dict(recipe.steps), + closing_steps=steps_to_dict(recipe.closing_steps), replaced_by_recipe_id=recipe.replaced_by_recipe_id, created_at=timestamps.created_at if timestamps is not None else None, versioned_at=timestamps.versioned_at if timestamps is not None else None, diff --git a/apps/api/src/cora/recipe/features/get_recipe/tool.py b/apps/api/src/cora/recipe/features/get_recipe/tool.py index e3a795ad3fa..ff3109ff406 100644 --- a/apps/api/src/cora/recipe/features/get_recipe/tool.py +++ b/apps/api/src/cora/recipe/features/get_recipe/tool.py @@ -34,6 +34,7 @@ class RecipeOutput(BaseModel): status: str version: str | None steps: dict[str, Any] + closing_steps: dict[str, Any] replaced_by_recipe_id: UUID | None created_at: datetime | None = None versioned_at: datetime | None = None @@ -73,6 +74,7 @@ async def get_recipe_tool( # pyright: ignore[reportUnusedFunction] status=recipe.status.value, version=recipe.version, steps=steps_to_dict(recipe.steps), + closing_steps=steps_to_dict(recipe.closing_steps), replaced_by_recipe_id=recipe.replaced_by_recipe_id, created_at=timestamps.created_at if timestamps is not None else None, versioned_at=timestamps.versioned_at if timestamps is not None else None, diff --git a/apps/api/src/cora/recipe/features/version_recipe/command.py b/apps/api/src/cora/recipe/features/version_recipe/command.py index 2390b3464d4..6297c6bba2e 100644 --- a/apps/api/src/cora/recipe/features/version_recipe/command.py +++ b/apps/api/src/cora/recipe/features/version_recipe/command.py @@ -19,8 +19,13 @@ @dataclass(frozen=True) class VersionRecipe: - """Issue a new version label + replacement steps for an existing Recipe.""" + """Issue a new version label + replacement steps for an existing Recipe. + + `closing_steps` is additive and optional (default `()`); it + REPLACES wholesale alongside `steps`. See `Recipe.closing_steps`. + """ recipe_id: UUID version_tag: str steps: tuple[RecipeStep, ...] + closing_steps: tuple[RecipeStep, ...] = () diff --git a/apps/api/src/cora/recipe/features/version_recipe/decider.py b/apps/api/src/cora/recipe/features/version_recipe/decider.py index 0d7eeffb80a..5c0bb701696 100644 --- a/apps/api/src/cora/recipe/features/version_recipe/decider.py +++ b/apps/api/src/cora/recipe/features/version_recipe/decider.py @@ -61,6 +61,7 @@ def decide( name=state.name, capability_id=state.capability_id, steps=command.steps, + closing_steps=command.closing_steps, ) return [ RecipeVersioned( @@ -68,5 +69,6 @@ def decide( version_tag=trimmed, steps=command.steps, occurred_at=now, + closing_steps=command.closing_steps, ) ] diff --git a/apps/api/src/cora/recipe/features/version_recipe/handler.py b/apps/api/src/cora/recipe/features/version_recipe/handler.py index 8bc540602f8..3f53717dfe7 100644 --- a/apps/api/src/cora/recipe/features/version_recipe/handler.py +++ b/apps/api/src/cora/recipe/features/version_recipe/handler.py @@ -116,9 +116,11 @@ async def handler( capability = await load_capability(deps.event_store, state.capability_id) if capability is None: raise CapabilityNotFoundError(state.capability_id) - validate_recipe_steps_against_capability_schema(command.steps, capability.parameters_schema) - validate_capture_refs(command.steps) - validate_output_refs(command.steps) + # ONE concatenated walk; see define_recipe/handler.py for why not two. + all_steps = command.steps + command.closing_steps + validate_recipe_steps_against_capability_schema(all_steps, capability.parameters_schema) + validate_capture_refs(all_steps) + validate_output_refs(all_steps) domain_events = decide(state=state, command=command, now=now) diff --git a/apps/api/src/cora/recipe/features/version_recipe/route.py b/apps/api/src/cora/recipe/features/version_recipe/route.py index 7d39b596df6..e304861a7ac 100644 --- a/apps/api/src/cora/recipe/features/version_recipe/route.py +++ b/apps/api/src/cora/recipe/features/version_recipe/route.py @@ -42,6 +42,13 @@ class VersionRecipeRequest(BaseModel): "are re-validated against the CURRENT Capability.parameters_schema." ), ) + closing_steps: dict[str, Any] = Field( + default_factory=lambda: {"steps": []}, + description=( + "Replacement closing-step sequence, same shape as `steps` " + "(wholesale replace alongside it). Empty by default." + ), + ) def _get_handler(request: Request) -> Handler: @@ -99,6 +106,7 @@ async def post_recipes_version( recipe_id=recipe_id, version_tag=body.version_tag, steps=steps_from_dict(body.steps), + closing_steps=steps_from_dict(body.closing_steps), ), principal_id=principal_id, correlation_id=cid, diff --git a/apps/api/src/cora/recipe/features/version_recipe/tool.py b/apps/api/src/cora/recipe/features/version_recipe/tool.py index 8dc2c6da6a8..74fcb4ee5f9 100644 --- a/apps/api/src/cora/recipe/features/version_recipe/tool.py +++ b/apps/api/src/cora/recipe/features/version_recipe/tool.py @@ -63,6 +63,17 @@ async def version_recipe_tool( # pyright: ignore[reportUnusedFunction] ), ), ], + closing_steps: Annotated[ + dict[str, Any] | None, + Field( + default=None, + description=( + "Optional replacement closing-step sequence, same shape " + "as `steps` (wholesale replace alongside it). Omit or " + "pass None for no closing steps." + ), + ), + ] = None, ) -> VersionRecipeOutput: handler = get_handler() await handler( @@ -70,6 +81,9 @@ async def version_recipe_tool( # pyright: ignore[reportUnusedFunction] recipe_id=recipe_id, version_tag=version_tag, steps=steps_from_dict(steps), + closing_steps=steps_from_dict( + closing_steps if closing_steps is not None else {"steps": []} + ), ), principal_id=get_mcp_principal_id(ctx), correlation_id=current_correlation_id(), diff --git a/apps/api/tests/architecture/test_conductor_result_construction_sites.py b/apps/api/tests/architecture/test_conductor_result_construction_sites.py index fe5671dd2c9..b6c22e98eb1 100644 --- a/apps/api/tests/architecture/test_conductor_result_construction_sites.py +++ b/apps/api/tests/architecture/test_conductor_result_construction_sites.py @@ -57,29 +57,54 @@ _REQUIRED_FIELDS = frozenset({"procedure_id", "completed_count"}) #: Pre-`start_procedure` lifecycle rejections: no step has run, so the entire -#: ledger is correctly empty. +#: ledger is correctly empty. `closing_failures` too: closing has not run. _PRE_START_OMISSIONS = frozenset( - {"actuation_kind", "artifacts", "held", "measurements", "outputs", "substrate_writes"} + { + "actuation_kind", + "artifacts", + "closing_failures", + "held", + "measurements", + "outputs", + "substrate_writes", + } ) #: Registry of every direct `ConductorResult(...)` site, keyed by #: (enclosing function name, 1-indexed occurrence within that function in #: source order), mapped to the EXACT set of fields it leaves at their #: default. See the module docstring for what this protects. +#: +#: `closing_failures` was added to EVERY entry below in one pass when the +#: field landed: none of these 14 sites runs `_run_closing` (that only +#: happens in the wrapper methods' terminal RETURN via `replace()`, which +#: this test never inspects), so every site correctly omits it. This is the +#: exact forcing function the module docstring describes -- the guard failed +#: at every registered site the moment the field existed, and the fix at +#: each site was "yes, still correctly omitted," not a code change. _EXPECTED_OMISSIONS: dict[tuple[str, int], frozenset[str]] = { # execute(): per-step and final results are built straight from local # data (the actuation observer, the compute accumulator, a running # count), never copied from a stale prior ConductorResult. `held` is - # correctly absent: execute() itself never holds anything. - ("execute", 1): frozenset({"held"}), - ("execute", 2): frozenset({"failure", "held"}), + # correctly absent: execute() itself never holds anything. Closing only + # ever runs from a wrapper's terminal branch, never inside execute(). + ("execute", 1): frozenset({"closing_failures", "held"}), + ("execute", 2): frozenset({"closing_failures", "failure", "held"}), # execute_from(): an ActionStep/ComputeStep halt-for-operator or a step # failure returns before any ComputeStep could run, so measurements / # artifacts / outputs are correctly empty; a resume replay never holds. - ("execute_from", 1): frozenset({"artifacts", "held", "measurements", "outputs"}), - ("execute_from", 2): frozenset({"artifacts", "held", "measurements", "outputs"}), - ("execute_from", 3): frozenset({"artifacts", "held", "measurements", "outputs"}), - ("execute_from", 4): frozenset({"artifacts", "failure", "held", "measurements"}), + ("execute_from", 1): frozenset( + {"artifacts", "closing_failures", "held", "measurements", "outputs"} + ), + ("execute_from", 2): frozenset( + {"artifacts", "closing_failures", "held", "measurements", "outputs"} + ), + ("execute_from", 3): frozenset( + {"artifacts", "closing_failures", "held", "measurements", "outputs"} + ), + ("execute_from", 4): frozenset( + {"artifacts", "closing_failures", "failure", "held", "measurements"} + ), # Pre-start lifecycle failures: start_procedure itself was rejected, so # no step ever ran. ("conduct", 1): _PRE_START_OMISSIONS, @@ -88,19 +113,22 @@ ("conduct_until_advised", 1): _PRE_START_OMISSIONS, # _abort_unconverged_cap / _abort_absolute_ceiling thread the last pass's # ledger through a None-safe ternary (2026-08-29 fix); `held` is the only - # remaining gap, correctly: neither loop-top abort ever holds. - ("_abort_unconverged_cap", 1): frozenset({"held"}), - ("_abort_absolute_ceiling", 1): frozenset({"held"}), + # remaining gap, correctly: neither loop-top abort ever holds. Closing + # steps are v1-refused for the loop slices, so closing_failures is + # correctly always empty here too. + ("_abort_unconverged_cap", 1): frozenset({"closing_failures", "held"}), + ("_abort_absolute_ceiling", 1): frozenset({"closing_failures", "held"}), # conduct_until_advised_from(): a frontier brain fault before any pass ran # (no execute() call yet, so no ledger to carry), and a resume-straight- # to-Stop synthetic placeholder fed into _complete_advised (same reason). ("conduct_until_advised_from", 1): frozenset( - {"artifacts", "held", "measurements", "outputs", "substrate_writes"} + {"artifacts", "closing_failures", "held", "measurements", "outputs", "substrate_writes"} ), ("conduct_until_advised_from", 2): frozenset( { "actuation_kind", "artifacts", + "closing_failures", "failure", "held", "measurements", diff --git a/apps/api/tests/architecture/test_recipe_evolver_carry_forward.py b/apps/api/tests/architecture/test_recipe_evolver_carry_forward.py index cf39ceb1617..8e22b95a61f 100644 --- a/apps/api/tests/architecture/test_recipe_evolver_carry_forward.py +++ b/apps/api/tests/architecture/test_recipe_evolver_carry_forward.py @@ -58,6 +58,8 @@ "capability_id": frozenset(), # A new version replaces the step list wholesale; that IS the event. "steps": frozenset({"RecipeVersioned"}), + # Same rationale: a version can fix a broken closing step too. + "closing_steps": frozenset({"RecipeVersioned"}), "version": frozenset({"RecipeVersioned"}), "replaced_by_recipe_id": frozenset({"RecipeDeprecated"}), } diff --git a/apps/api/tests/contract/test_conduct_from_procedure_endpoint.py b/apps/api/tests/contract/test_conduct_from_procedure_endpoint.py index 378a720bd71..1a2c9ebbc1e 100644 --- a/apps/api/tests/contract/test_conduct_from_procedure_endpoint.py +++ b/apps/api/tests/contract/test_conduct_from_procedure_endpoint.py @@ -172,6 +172,74 @@ def test_post_conduct_from_returns_400_for_boundary_past_step_count() -> None: assert response.status_code == 400 +def _register_from_recipe_with_closing_capture(client: TestClient) -> UUID: + """Recipe whose closing step reads a capture only its SECOND main step + declares. The failing setpoint at index 0 is recoverable (Held); the + CaptureStep at index 1 never runs during that pass but is still part of + the pinned resolved steps.""" + cap = client.post( + "/capabilities", + json={ + "code": "cora.capability.closing_capture_recipe", + "name": "ClosingCaptureCap", + "required_affordances": [], + "executor_shapes": ["Procedure"], + }, + ).json() + recipe = client.post( + "/recipes", + json={ + "name": "closing capture recipe", + "capability_id": cap["capability_id"], + "steps": { + "steps": [ + {"kind": "setpoint", "address": "2bma:unconnected", "value": 1.0}, + {"kind": "capture", "address": "2bma:readback", "capture_name": "a_readback"}, + ], + }, + "closing_steps": { + "steps": [ + { + "kind": "setpoint", + "address": "2bma:shutter", + "value": {"__capture__": "a_readback"}, + }, + ], + }, + }, + ).json() + registered = client.post( + "/procedures/from-recipe", + json={ + "name": "closing capture procedure", + "kind": "bakeout", + "target_asset_ids": [], + "parent_run_id": None, + "recipe_id": recipe["recipe_id"], + "bindings": {}, + }, + ) + assert registered.status_code == 201, registered.text + return UUID(registered.json()["procedure_id"]) + + +@pytest.mark.contract +def test_post_conduct_from_returns_422_for_closing_capture_before_boundary() -> None: + """A closing CaptureRef naming a capture only a pre-boundary main step + declares is rejected up front: resume starts captures empty, so a + boundary that skips the declaring step would never populate it.""" + with TestClient(create_app()) as client: + pid = _register_from_recipe_with_closing_capture(client) + held = client.post(f"/procedures/{pid}/conduct-or-hold", json={"steps": []}) + assert held.status_code == 200, held.text + assert held.json()["held"] is True + # boundary == 2 (both main steps done): skips the CaptureStep entirely. + response = client.post( + f"/procedures/{pid}/conduct-from", json={"re_establishment_boundary": 2} + ) + assert response.status_code == 422, response.text + + @pytest.mark.contract def test_post_conduct_from_aborts_on_a_genuine_step_failure() -> None: """Replaying a tail whose setpoint still fails (unconnected address) aborts: diff --git a/apps/api/tests/contract/test_conduct_until_advised_endpoint.py b/apps/api/tests/contract/test_conduct_until_advised_endpoint.py index b31b3902e25..52a21ff7a34 100644 --- a/apps/api/tests/contract/test_conduct_until_advised_endpoint.py +++ b/apps/api/tests/contract/test_conduct_until_advised_endpoint.py @@ -191,6 +191,76 @@ def _register_from_steered_recipe(client: TestClient) -> UUID: return UUID(registered.json()["procedure_id"]) +def _register_from_steered_recipe_with_closing_steps(client: TestClient) -> UUID: + """Same as `_register_from_steered_recipe`, plus a non-empty closing_steps + on the Recipe -- the shape conduct_until_advised must refuse (v1 scope: + a loop that re-walks one pass block has no defined place to run + _run_closing).""" + cap = client.post( + "/capabilities", + json={ + "code": "cora.capability.steered_align_recipe_closing", + "name": "SteeredAlignClosingCap", + "required_affordances": [], + "executor_shapes": ["Method", "Procedure"], + }, + ).json() + recipe = client.post( + "/recipes", + json={ + "name": "steered align recipe with closing steps", + "capability_id": cap["capability_id"], + "steps": { + "steps": [ + { + "kind": "compute", + "command": ["tomopy", "find_center"], + "input_uris": ["file:///data/19bm/align/theta_pair.h5"], + "output_uri": None, + "parameters": {}, + "capture_name": _OBJECTIVE_NAME, + }, + { + "kind": "setpoint", + "address": "19bm:sample_rotary_theta", + "value": {"__steering__": _AXIS}, + "verify": False, + }, + ], + }, + "closing_steps": { + "steps": [ + {"kind": "setpoint", "address": "19bm:shutter", "value": 0.0}, + ], + }, + }, + ).json() + registered = client.post( + "/procedures/from-recipe", + json={ + "name": "steered align from recipe with closing", + "kind": "rotation_center_characterization", + "target_asset_ids": [], + "parent_run_id": None, + "recipe_id": recipe["recipe_id"], + "bindings": {}, + }, + ) + assert registered.status_code == 201, registered.text + return UUID(registered.json()["procedure_id"]) + + +@pytest.mark.contract +def test_post_conduct_until_advised_refuses_a_closing_bearing_recipe() -> None: + """v1 scope: a loop-driving slice has no defined place to run + _run_closing, so it refuses a closing-bearing Recipe outright (422), + before any FSM event fires.""" + with TestClient(create_app()) as client: + pid = _register_from_steered_recipe_with_closing_steps(client) + run = client.post(_PATH.format(pid=pid), json=_body()) + assert run.status_code == 422, run.text + + @pytest.mark.contract def test_post_conduct_until_advised_steered_recipe_executes_over_the_wire() -> None: """A SteeringRef-authored recipe drives conduct_until_advised over the wire. diff --git a/apps/api/tests/contract/test_get_recipe_endpoint.py b/apps/api/tests/contract/test_get_recipe_endpoint.py index d1d27cc1411..5c4505fdaa9 100644 --- a/apps/api/tests/contract/test_get_recipe_endpoint.py +++ b/apps/api/tests/contract/test_get_recipe_endpoint.py @@ -44,6 +44,32 @@ def test_get_recipe_200_returns_full_recipe_response() -> None: assert body["steps"]["steps"][0]["kind"] == "setpoint" +@pytest.mark.contract +def test_get_recipe_surfaces_closing_steps() -> None: + with TestClient(create_app()) as client: + cap = client.post("/capabilities", json=_capability()).json() + body = _recipe_for(cap["capability_id"]) + body["closing_steps"] = { + "steps": [ + {"kind": "setpoint", "address": "dev:shutter", "value": 0.0, "verify": False}, + ], + } + recipe = client.post("/recipes", json=body).json() + response = client.get(f"/recipes/{recipe['recipe_id']}") + assert response.status_code == 200 + resp_body = response.json() + assert resp_body["closing_steps"]["steps"][0]["address"] == "dev:shutter" + + +@pytest.mark.contract +def test_get_recipe_defaults_closing_steps_to_empty() -> None: + with TestClient(create_app()) as client: + cap = client.post("/capabilities", json=_capability()).json() + recipe = client.post("/recipes", json=_recipe_for(cap["capability_id"])).json() + response = client.get(f"/recipes/{recipe['recipe_id']}") + assert response.json()["closing_steps"]["steps"] == [] + + @pytest.mark.contract def test_get_recipe_404_when_recipe_missing() -> None: with TestClient(create_app()) as client: diff --git a/apps/api/tests/contract/test_get_recipe_mcp_tool.py b/apps/api/tests/contract/test_get_recipe_mcp_tool.py index 9d5697b49f7..4653fe45f14 100644 --- a/apps/api/tests/contract/test_get_recipe_mcp_tool.py +++ b/apps/api/tests/contract/test_get_recipe_mcp_tool.py @@ -78,3 +78,21 @@ def test_mcp_get_recipe_tool_returns_structured_recipe_state() -> None: assert body["id"] == recipe_id assert body["status"] == "Defined" assert body["capability_id"] == capability_id + + +@pytest.mark.contract +def test_mcp_get_recipe_tool_surfaces_closing_steps_defined_via_mcp() -> None: + with TestClient(create_app()) as client: + session_headers = open_session(client) + cap_result = _call_tool(client, session_headers, "define_capability", _capability_args(), 2) + capability_id = cap_result["structuredContent"]["capability_id"] + args = _recipe_args(capability_id) + args["closing_steps"] = { + "steps": [{"kind": "setpoint", "address": "dev:shutter", "value": 0.0}], + } + recipe_result = _call_tool(client, session_headers, "define_recipe", args, 3) + recipe_id = recipe_result["structuredContent"]["recipe_id"] + result = _call_tool(client, session_headers, "get_recipe", {"recipe_id": recipe_id}, 4) + assert result["isError"] is False + body = result["structuredContent"] + assert body["closing_steps"]["steps"][0]["address"] == "dev:shutter" diff --git a/apps/api/tests/unit/operation/test_conduct_from_procedure_handler.py b/apps/api/tests/unit/operation/test_conduct_from_procedure_handler.py index 5ff999a6193..ca4d0292fc5 100644 --- a/apps/api/tests/unit/operation/test_conduct_from_procedure_handler.py +++ b/apps/api/tests/unit/operation/test_conduct_from_procedure_handler.py @@ -38,8 +38,15 @@ load_procedure, to_payload, ) -from cora.operation.conductor import ActionStep, Conductor, SetpointStep, Step, step_to_payload -from cora.operation.errors import UnauthorizedError +from cora.operation.conductor import ( + ActionStep, + CaptureStep, + Conductor, + SetpointStep, + Step, + step_to_payload, +) +from cora.operation.errors import ClosingCaptureBeforeBoundaryError, UnauthorizedError from cora.operation.features import ( abort_procedure, append_activities, @@ -55,6 +62,8 @@ Handler as ConductFromHandler, ) from cora.operation.ports.control_port import ActuationKind, ControlPort +from cora.operation.ports.measurement import Measurement +from cora.recipe.aggregates.recipe.body import CaptureRef from cora.run.aggregates.run import RunHeld, RunStarted from cora.run.aggregates.run import event_type_name as run_event_type_name from cora.run.aggregates.run import to_payload as run_to_payload @@ -100,6 +109,7 @@ async def _seed_held_with_steps( store: InMemoryEventStore, *, steps: Sequence[Step], + closing_steps: Sequence[Step] = (), procedure_id: UUID = _PROCEDURE_ID, parent_run_id: UUID | None = None, held_actuation_kind: str | None = None, @@ -108,6 +118,7 @@ async def _seed_held_with_steps( (the pinned resolved steps) + Started + Held. `held_actuation_kind` is the kind the pre-hold conduct observed (carried on ProcedureHeld).""" resolved = tuple(step_to_payload(s) for s in steps) + resolved_closing = tuple(step_to_payload(s) for s in closing_steps) events = [ ProcedureRegistered( procedure_id=procedure_id, @@ -120,7 +131,8 @@ async def _seed_held_with_steps( ResolvedStepsRecorded( procedure_id=procedure_id, resolved_steps=resolved, - step_count=len(resolved), + resolved_closing_steps=resolved_closing, + step_count=len(resolved) + len(resolved_closing), occurred_at=_PRIOR, ), ProcedureStarted(procedure_id=procedure_id, occurred_at=_PRIOR), @@ -182,6 +194,10 @@ async def _status(store: InMemoryEventStore) -> ProcedureStatus: return state.status +def _good_reading(value: float) -> Measurement: + return Measurement(value=value, kind="Scalar", quality="Good", produced_at=_NOW) + + async def _call(handler: ConductFromHandler, boundary: int) -> ConductFromProcedureResult: return await handler( ConductFromProcedure(procedure_id=_PROCEDURE_ID, re_establishment_boundary=boundary), @@ -444,3 +460,65 @@ async def test_conduct_from_folds_pre_hold_actuation_kind_into_completion() -> N assert state is not None assert state.status is ProcedureStatus.COMPLETED assert state.actuation_kind == ActuationKind.HYBRID.value + + +@pytest.mark.unit +async def test_clean_tail_resume_also_runs_pinned_closing_steps() -> None: + """resolved_closing_steps is read off the pinned record and handed to + Conductor.conduct_from, not just parsed and discarded.""" + store = InMemoryEventStore() + port = InMemoryControlPort() + port.simulate_connect("2bma:a") + port.simulate_connect("2bma:shutter") + await _seed_held_with_steps( + store, + steps=(SetpointStep(address="2bma:a", value=1.0),), + closing_steps=(SetpointStep(address="2bma:shutter", value=0.0),), + ) + deps = _deps(store) + result = await _call(_make_conduct_from(deps, port), 0) + + assert result.succeeded is True + assert (await port.read("2bma:shutter")).value == 0.0 + assert result.substrate_writes == {"2bma:a": 1.0, "2bma:shutter": 0.0} + + +@pytest.mark.unit +async def test_raises_closing_capture_before_boundary_when_only_prefix_declares_it() -> None: + """A closing CaptureRef naming a capture only a PRE-boundary main step + declares would resolve against nothing (captures start empty on resume) -- + rejected up front rather than surfacing as a closing_failures entry.""" + store = InMemoryEventStore() + await _seed_held_with_steps( + store, + steps=( + CaptureStep(address="2bma:a", capture_name="a_readback"), + SetpointStep(address="2bma:b", value=2.0), + ), + closing_steps=(SetpointStep(address="2bma:shutter", value=CaptureRef("a_readback")),), + ) + deps = _deps(store) + with pytest.raises(ClosingCaptureBeforeBoundaryError) as exc: + await _call(_make_conduct_from(deps, InMemoryControlPort()), 1) # skips the CaptureStep + assert exc.value.capture_name == "a_readback" + assert exc.value.boundary == 1 + + +@pytest.mark.unit +async def test_closing_capture_declared_at_boundary_is_accepted() -> None: + """The same capture_name, declared AT the boundary (re-run this resume), + is fine: it will be populated before the closing step reads it.""" + store = InMemoryEventStore() + port = InMemoryControlPort() + port.set_reading("2bma:a", _good_reading(1.0)) + port.simulate_connect("2bma:shutter") + await _seed_held_with_steps( + store, + steps=(CaptureStep(address="2bma:a", capture_name="a_readback"),), + closing_steps=(SetpointStep(address="2bma:shutter", value=CaptureRef("a_readback")),), + ) + deps = _deps(store) + result = await _call(_make_conduct_from(deps, port), 0) + + assert result.succeeded is True + assert (await port.read("2bma:shutter")).value == 1.0 diff --git a/apps/api/tests/unit/operation/test_conduct_or_hold_procedure_handler.py b/apps/api/tests/unit/operation/test_conduct_or_hold_procedure_handler.py index a3971dcc27e..ae9a7033192 100644 --- a/apps/api/tests/unit/operation/test_conduct_or_hold_procedure_handler.py +++ b/apps/api/tests/unit/operation/test_conduct_or_hold_procedure_handler.py @@ -13,6 +13,7 @@ - unknown procedure -> ProcedureNotFoundError """ +import hashlib from collections.abc import Sequence from dataclasses import dataclass from datetime import UTC, datetime @@ -24,6 +25,7 @@ from cora.infrastructure.event_envelope import to_new_event from cora.infrastructure.kernel import Kernel from cora.infrastructure.routing import NIL_SENTINEL_ID +from cora.operation._recipe_expansion import steps_to_wire_with_closing from cora.operation.adapters.in_memory_control_port import InMemoryControlPort from cora.operation.adapters.in_memory_recipe_expander import InMemoryRecipeExpander from cora.operation.aggregates.procedure import ( @@ -32,6 +34,7 @@ ProcedureRegistered, ProcedureStarted, ProcedureStatus, + RecipeExpansionRecorded, event_type_name, load_procedure, to_payload, @@ -62,6 +65,10 @@ Handler as ConductOrHoldHandler, ) from cora.operation.features.hold_procedure.command import HoldProcedure +from cora.recipe.aggregates.recipe import RecipeDefined, RecipeSetpointStep +from cora.recipe.aggregates.recipe import event_type_name as recipe_event_type_name +from cora.recipe.aggregates.recipe import to_payload as recipe_to_payload +from cora.shared.canonical_json import canonical_json_bytes from tests.unit._helpers import build_deps as _build_deps_shared _NOW = datetime(2026, 6, 21, 12, 0, 0, tzinfo=UTC) @@ -70,6 +77,110 @@ _CORRELATION_ID = UUID("01900000-0000-7000-8000-0000000000aa") +# --- recipe-driven closing_steps wiring --------------------------------- +# +# A single test proving `resolve_and_pin_conduct_steps`'s returned +# `closing_steps` actually reaches `Conductor.conduct_or_hold`, not just +# `ResolvedStepsRecorded` (a real Conductor + real port catches a dropped +# kwarg a fake conductor's call-recording would also catch, but this pins +# the ACTUAL closing walk executing, matching this file's real-Conductor +# style for everything else). + + +async def _seed_recipe_driven_defined( + store: InMemoryEventStore, + *, + recipe_steps: tuple[RecipeSetpointStep, ...], + recipe_closing_steps: tuple[RecipeSetpointStep, ...], +) -> None: + """Seed a recipe-driven, Defined Procedure: a Recipe stream + the + ProcedureRegistered + RecipeExpansionRecorded genesis block + `register_procedure_from_recipe` emits. No Capability stream needed: + `load_capability` returning None (unseeded) is treated as "not + deprecated", the same as a real, non-deprecated Capability.""" + recipe_id = uuid4() + capability_id = uuid4() + bindings: dict[str, object] = {} + recipe_event = RecipeDefined( + recipe_id=recipe_id, + name="R", + capability_id=capability_id, + steps=recipe_steps, + closing_steps=recipe_closing_steps, + occurred_at=_NOW, + ) + await store.append( + stream_type="Recipe", + stream_id=recipe_id, + expected_version=0, + events=[ + to_new_event( + event_type=recipe_event_type_name(recipe_event), + payload=recipe_to_payload(recipe_event), + occurred_at=_NOW, + event_id=uuid4(), + command_name="seed", + correlation_id=_CORRELATION_ID, + principal_id=_PRINCIPAL_ID, + ), + ], + ) + expanded: tuple[Step, ...] = tuple( + SetpointStep(address=s.address, value=s.value) # type: ignore[arg-type] + for s in recipe_steps + ) + expanded_closing: tuple[Step, ...] = tuple( + SetpointStep(address=s.address, value=s.value) # type: ignore[arg-type] + for s in recipe_closing_steps + ) + steps_hash = hashlib.sha256( + canonical_json_bytes(steps_to_wire_with_closing(expanded, expanded_closing)) + ).hexdigest() + bindings_hash = hashlib.sha256(canonical_json_bytes(bindings)).hexdigest() + events = [ + ProcedureRegistered( + procedure_id=_PROCEDURE_ID, + name="P", + kind="bakeout", + target_asset_ids=(), + parent_run_id=None, + capability_id=capability_id, + recipe_id=recipe_id, + occurred_at=_NOW, + ), + RecipeExpansionRecorded( + procedure_id=_PROCEDURE_ID, + recipe_id=recipe_id, + recipe_version=None, + capability_id=capability_id, + capability_version=None, + bindings=bindings, + expansion_port_version="v2-pseudoaxis-aware", + steps_hash=steps_hash, + bindings_hash=bindings_hash, + step_count=len(recipe_steps) + len(recipe_closing_steps), + occurred_at=_NOW, + ), + ] + await store.append( + stream_type="Procedure", + stream_id=_PROCEDURE_ID, + expected_version=0, + events=[ + to_new_event( + event_type=event_type_name(event), # type: ignore[arg-type] + payload=to_payload(event), # type: ignore[arg-type] + occurred_at=_NOW, + event_id=uuid4(), + command_name="seed", + correlation_id=_CORRELATION_ID, + principal_id=_PRINCIPAL_ID, + ) + for event in events + ], + ) + + @dataclass class _LenientIds: """Conductor id_generator that never exhausts (markers double appends).""" @@ -395,3 +506,48 @@ async def test_complete_rejected_records_lifecycle_failure() -> None: assert result.failure is not None assert result.failure.source_kind == "lifecycle" assert result.failure.target == "complete" + + +@pytest.mark.unit +async def test_recipe_driven_clean_run_also_runs_pinned_closing_steps() -> None: + """resolve_and_pin_conduct_steps's returned closing_steps must reach + Conductor.conduct_or_hold, not just ResolvedStepsRecorded -- a dropped + kwarg here would leave every recipe's closing steps silently unwalked.""" + store = InMemoryEventStore() + port = InMemoryControlPort() + port.simulate_connect("2bma:a") + port.simulate_connect("2bma:shutter") + await _seed_recipe_driven_defined( + store, + recipe_steps=(RecipeSetpointStep(address="2bma:a", value=1.0),), + recipe_closing_steps=(RecipeSetpointStep(address="2bma:shutter", value=0.0),), + ) + result = await _call(_make_conduct_or_hold(_deps(store), port), ()) + + assert result.succeeded is True + assert result.held is False + assert await _status(store) is ProcedureStatus.COMPLETED + assert (await port.read("2bma:shutter")).value == 0.0 + assert result.substrate_writes == {"2bma:a": 1.0, "2bma:shutter": 0.0} + assert result.closing_failures == () + + +@pytest.mark.unit +async def test_recipe_driven_clean_run_isolates_a_failing_closing_step() -> None: + """closing_failures threads onto ConductOrHoldProcedureResult -- a + failing closing step (unconnected shutter address) is isolated: it + neither flips `succeeded` nor blocks the main walk's own success.""" + store = InMemoryEventStore() + port = InMemoryControlPort() + port.simulate_connect("2bma:a") # shutter left unconnected: the closing write fails + await _seed_recipe_driven_defined( + store, + recipe_steps=(RecipeSetpointStep(address="2bma:a", value=1.0),), + recipe_closing_steps=(RecipeSetpointStep(address="2bma:shutter", value=0.0),), + ) + result = await _call(_make_conduct_or_hold(_deps(store), port), ()) + + assert result.succeeded is True + assert await _status(store) is ProcedureStatus.COMPLETED + assert len(result.closing_failures) == 1 + assert result.closing_failures[0].error_class == "ControlNotConnectedError" diff --git a/apps/api/tests/unit/operation/test_conduct_procedure_handler.py b/apps/api/tests/unit/operation/test_conduct_procedure_handler.py index d57829ab94e..a2ea988b8b9 100644 --- a/apps/api/tests/unit/operation/test_conduct_procedure_handler.py +++ b/apps/api/tests/unit/operation/test_conduct_procedure_handler.py @@ -126,6 +126,7 @@ class _ConductCall: causation_id: UUID | None surface_id: UUID steps: Sequence[Step] + closing_steps: Sequence[Step] = () @dataclass @@ -142,6 +143,7 @@ async def conduct( principal_id: UUID, correlation_id: UUID, steps: Sequence[Step], + closing_steps: Sequence[Step] = (), causation_id: UUID | None = None, surface_id: UUID = NIL_SENTINEL_ID, ) -> ConductorResult: @@ -153,6 +155,7 @@ async def conduct( causation_id=causation_id, surface_id=surface_id, steps=steps, + closing_steps=closing_steps, ) ) return self.result @@ -503,7 +506,7 @@ def test_conduct_procedure_request_default_is_empty_step_list() -> None: import hashlib # noqa: E402 -from cora.operation._recipe_expansion import steps_to_wire # noqa: E402 +from cora.operation._recipe_expansion import steps_to_wire_with_closing # noqa: E402 from cora.operation.aggregates.procedure import ( # noqa: E402 ProcedureBoundCapabilityDeprecatedError, ProcedureNotFoundError, @@ -599,6 +602,7 @@ async def _seed_recipe_driven_procedure( *, bindings: dict[str, object] | None = None, recipe_steps: tuple[RecipeSetpointStep, ...] | None = None, + recipe_closing_steps: tuple[RecipeSetpointStep, ...] = (), expansion_port_version: str = "v2-pseudoaxis-aware", bindings_hash_override: str | None = None, steps_hash_override: str | None = None, @@ -625,6 +629,7 @@ async def _seed_recipe_driven_procedure( capability_id=capability_id, steps=rsteps, occurred_at=_NOW, + closing_steps=recipe_closing_steps, ) await store.append( stream_type="Recipe", @@ -653,8 +658,14 @@ async def _seed_recipe_driven_procedure( SetpointStep(address=s.address, value=s.value) # type: ignore[arg-type] for s in rsteps ) + expanded_closing_for_hash: tuple[Step, ...] = tuple( + SetpointStep(address=s.address, value=s.value) # type: ignore[arg-type] + for s in recipe_closing_steps + ) expected_steps_hash = hashlib.sha256( - canonical_json_bytes(steps_to_wire(expanded_for_hash)) + canonical_json_bytes( + steps_to_wire_with_closing(expanded_for_hash, expanded_closing_for_hash) + ) ).hexdigest() registered = ProcedureRegistered( procedure_id=procedure_id, @@ -678,7 +689,7 @@ async def _seed_recipe_driven_procedure( expansion_port_version=expansion_port_version, steps_hash=steps_hash_override or expected_steps_hash, bindings_hash=bindings_hash_override or expected_bindings_hash, - step_count=len(rsteps), + step_count=len(rsteps) + len(recipe_closing_steps), occurred_at=_NOW, ) procedure_events.append(recorded) # type: ignore[arg-type] @@ -852,6 +863,70 @@ async def test_recipe_driven_handler_with_steps_drift_raises_steps_mismatch() -> assert exc.value.mismatch_field == "steps" +@pytest.mark.unit +async def test_conduct_procedure_recipe_with_closing_steps_pins_them_re_expanded() -> None: + """A recipe-driven conduct re-expands recipe.closing_steps too, and pins + the result onto ResolvedStepsRecorded.resolved_closing_steps.""" + procedure_id = uuid4() + recipe_id = uuid4() + store = InMemoryEventStore() + await _seed_recipe_driven_procedure( + store, + procedure_id, + recipe_id, + recipe_closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + ) + conductor = _FakeConductor(result=ConductorResult(procedure_id=procedure_id, completed_count=1)) + handler = _bind_handler(store, conductor) + + await handler( + ConductProcedure(procedure_id=procedure_id, steps=()), + principal_id=uuid4(), + correlation_id=uuid4(), + ) + + stored, _ = await store.load(stream_type="Procedure", stream_id=procedure_id) + recorded = [ + event + for event in (from_stored(s) for s in stored) + if isinstance(event, ResolvedStepsRecorded) + ] + assert len(recorded) == 1 + expected_closing = SetpointStep(address="dev:shutter", value=0.0) + assert recorded[0].resolved_closing_steps == (step_to_payload(expected_closing),) + # Pinned is not enough: the Conductor must actually receive them, or + # _run_closing never walks them. + assert conductor.calls[0].closing_steps == (expected_closing,) + assert recorded[0].step_count == 2 # 1 main + 1 closing + + +@pytest.mark.unit +async def test_recipe_driven_handler_with_closing_steps_present_still_verifies_the_hash() -> None: + """A wrong pinned hash must still be caught when closing_steps is + non-empty -- proves threading closing_steps through verify_steps_hash + does not accidentally short-circuit verification.""" + procedure_id = uuid4() + recipe_id = uuid4() + store = InMemoryEventStore() + await _seed_recipe_driven_procedure( + store, + procedure_id, + recipe_id, + recipe_closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + steps_hash_override="0" * 64, + ) + conductor = _FakeConductor(result=ConductorResult(procedure_id=procedure_id, completed_count=0)) + handler = _bind_handler(store, conductor) + with pytest.raises(RecipeExpansionReplayMismatchError) as exc: + await handler( + ConductProcedure(procedure_id=procedure_id, steps=()), + principal_id=uuid4(), + correlation_id=uuid4(), + ) + assert exc.value.procedure_id == procedure_id + assert exc.value.mismatch_field == "steps" + + @pytest.mark.unit async def test_conduct_procedure_with_unregistered_procedure_raises_procedure_not_found_error() -> ( None diff --git a/apps/api/tests/unit/operation/test_conduct_until_advised_from_handler.py b/apps/api/tests/unit/operation/test_conduct_until_advised_from_handler.py index 88491b2e294..015a6e0125d 100644 --- a/apps/api/tests/unit/operation/test_conduct_until_advised_from_handler.py +++ b/apps/api/tests/unit/operation/test_conduct_until_advised_from_handler.py @@ -67,7 +67,7 @@ Step, step_to_payload, ) -from cora.operation.errors import UnauthorizedError +from cora.operation.errors import UnauthorizedError, UnsupportedClosingStepsError from cora.operation.features import ( abort_procedure, append_activities, @@ -217,6 +217,7 @@ async def _seed_held_steered( open_pass: bool = False, extra_outcome: tuple[int, float, float] | None = None, procedure_id: UUID = _PROCEDURE_ID, + resolved_closing_steps: tuple[Step, ...] = (), ) -> None: """Land a conducted-then-Held steered Procedure with recorded closed passes. @@ -233,6 +234,7 @@ async def _seed_held_steered( open pass. It is recovered + re-fed by the resume. """ resolved = tuple(step_to_payload(s) for s in _steered_block()) + resolved_closing = tuple(step_to_payload(s) for s in resolved_closing_steps) events: list[ProcedureEvent] = [ ProcedureRegistered( procedure_id=procedure_id, @@ -245,7 +247,8 @@ async def _seed_held_steered( ResolvedStepsRecorded( procedure_id=procedure_id, resolved_steps=resolved, - step_count=len(resolved), + resolved_closing_steps=resolved_closing, + step_count=len(resolved) + len(resolved_closing), occurred_at=_PRIOR, ), ProcedureStarted(procedure_id=procedure_id, occurred_at=_PRIOR), @@ -624,6 +627,28 @@ async def test_resume_authz_deny_raises_unauthorized() -> None: with pytest.raises(UnauthorizedError): await _call(_make_conduct_from(deps, port, compute, outcome_store)) + + +@pytest.mark.unit +async def test_resume_refuses_a_closing_bearing_pinned_record() -> None: + """v1 scope, mirroring conduct_until_advised's forward-direction refusal: + the pinned record already carries resolved_closing_steps when the bound + Recipe has any, so no fresh Recipe load is needed to reject here.""" + store = InMemoryEventStore() + port = InMemoryControlPort() + port.simulate_connect(_MOTOR_ADDR) + compute = InMemoryComputePort() + outcome_store = InMemoryOutcomeStore() + await _seed_held_steered( + store, + outcome_store, + closed=[(3.0, 2.0)], + resolved_closing_steps=(SetpointStep(address="2bma:shutter", value=0.0),), + ) + deps = _deps(store) + + with pytest.raises(UnsupportedClosingStepsError): + await _call(_make_conduct_from(deps, port, compute, outcome_store)) assert await _status(store) is ProcedureStatus.HELD diff --git a/apps/api/tests/unit/operation/test_conduct_until_converged_handler.py b/apps/api/tests/unit/operation/test_conduct_until_converged_handler.py index fd5480cb000..02b994c3b2a 100644 --- a/apps/api/tests/unit/operation/test_conduct_until_converged_handler.py +++ b/apps/api/tests/unit/operation/test_conduct_until_converged_handler.py @@ -13,6 +13,7 @@ - result_to_wire serializes success + failure """ +import hashlib from collections.abc import Sequence from dataclasses import dataclass, field from datetime import UTC, datetime @@ -26,9 +27,11 @@ from cora.infrastructure.ports.clock import FakeClock from cora.infrastructure.ports.id_generator import UUIDv7Generator from cora.infrastructure.routing import NIL_SENTINEL_ID +from cora.operation._recipe_expansion import steps_to_wire_with_closing from cora.operation.adapters.in_memory_recipe_expander import InMemoryRecipeExpander from cora.operation.aggregates.procedure import ( ProcedureRegistered, + RecipeExpansionRecorded, event_type_name, to_payload, ) @@ -40,18 +43,115 @@ Step, WithinToleranceCriterion, ) -from cora.operation.errors import UnauthorizedError +from cora.operation.errors import UnauthorizedError, UnsupportedClosingStepsError from cora.operation.features.conduct_until_converged.command import ( ConductUntilConverged, ConductUntilConvergedResult, ) from cora.operation.features.conduct_until_converged.handler import bind from cora.operation.features.conduct_until_converged.route import result_to_wire +from cora.recipe.aggregates.recipe import RecipeDefined, RecipeSetpointStep +from cora.recipe.aggregates.recipe import event_type_name as recipe_event_type_name +from cora.recipe.aggregates.recipe import to_payload as recipe_to_payload +from cora.shared.canonical_json import canonical_json_bytes _NOW = datetime(2026, 6, 24, 12, 0, 0, tzinfo=UTC) _CRITERION = WithinToleranceCriterion(expected=0.0, tolerance=0.5) +async def _seed_recipe_driven_procedure_with_closing_steps( + store: InMemoryEventStore, procedure_id: UUID +) -> None: + """Seed a recipe-driven, Defined Procedure whose Recipe carries a + non-empty closing_steps -- the shape conduct_until_converged must + refuse (v1 scope). No Capability stream needed: `load_capability` + returning None is treated as "not deprecated".""" + recipe_id = uuid4() + capability_id = uuid4() + recipe_steps = (RecipeSetpointStep(address="dev:x", value=1.0),) + recipe_closing_steps = (RecipeSetpointStep(address="dev:shutter", value=0.0),) + recipe_event = RecipeDefined( + recipe_id=recipe_id, + name="R", + capability_id=capability_id, + steps=recipe_steps, + closing_steps=recipe_closing_steps, + occurred_at=_NOW, + ) + await store.append( + stream_type="Recipe", + stream_id=recipe_id, + expected_version=0, + events=[ + to_new_event( + event_type=recipe_event_type_name(recipe_event), + payload=recipe_to_payload(recipe_event), + occurred_at=_NOW, + event_id=uuid4(), + command_name="seed", + correlation_id=uuid4(), + causation_id=None, + principal_id=uuid4(), + ), + ], + ) + expanded: tuple[Step, ...] = tuple( + SetpointStep(address=s.address, value=s.value) # type: ignore[arg-type] + for s in recipe_steps + ) + expanded_closing: tuple[Step, ...] = tuple( + SetpointStep(address=s.address, value=s.value) # type: ignore[arg-type] + for s in recipe_closing_steps + ) + steps_hash = hashlib.sha256( + canonical_json_bytes(steps_to_wire_with_closing(expanded, expanded_closing)) + ).hexdigest() + bindings_hash = hashlib.sha256(canonical_json_bytes({})).hexdigest() + events = [ + ProcedureRegistered( + procedure_id=procedure_id, + name="P", + kind="bakeout", + target_asset_ids=(), + parent_run_id=None, + capability_id=capability_id, + recipe_id=recipe_id, + occurred_at=_NOW, + ), + RecipeExpansionRecorded( + procedure_id=procedure_id, + recipe_id=recipe_id, + recipe_version=None, + capability_id=capability_id, + capability_version=None, + bindings={}, + expansion_port_version="v2-pseudoaxis-aware", + steps_hash=steps_hash, + bindings_hash=bindings_hash, + step_count=len(recipe_steps) + len(recipe_closing_steps), + occurred_at=_NOW, + ), + ] + await store.append( + stream_type="Procedure", + stream_id=procedure_id, + expected_version=0, + events=[ + to_new_event( + event_type=event_type_name(event), # type: ignore[arg-type] + payload=to_payload(event), # type: ignore[arg-type] + occurred_at=_NOW, + event_id=uuid4(), + command_name="seed", + correlation_id=uuid4(), + causation_id=None, + principal_id=uuid4(), + ) + for event in events + ], + ) + + async def _seed_procedure( store: InMemoryEventStore, procedure_id: UUID, @@ -281,3 +381,31 @@ def test_result_to_wire_serializes_cap_abort_failure() -> None: assert wire.failure is not None assert wire.failure.error_class == "ConvergenceIterationCapReached" assert wire.failure.step_index is None + + +@pytest.mark.unit +async def test_handler_refuses_a_closing_bearing_recipe() -> None: + """v1 scope: a loop-driving slice has no defined place to run + _run_closing, so it refuses rather than silently dropping the Recipe's + closing steps. The Conductor is never invoked.""" + procedure_id = uuid4() + store = InMemoryEventStore() + await _seed_recipe_driven_procedure_with_closing_steps(store, procedure_id) + conductor = _FakeConductor(result=ConductorResult(procedure_id=procedure_id, completed_count=0)) + handler = bind( + _deps(_FakeAuthz(), store), # type: ignore[arg-type] + conductor=conductor, # type: ignore[arg-type] + expansion_port=InMemoryRecipeExpander(), + ) + with pytest.raises(UnsupportedClosingStepsError) as exc: + await handler( + ConductUntilConverged( + procedure_id=procedure_id, + convergence_capture_name="offset", + criterion=_CRITERION, + ), + principal_id=uuid4(), + correlation_id=uuid4(), + ) + assert exc.value.procedure_id == procedure_id + assert conductor.calls == [] diff --git a/apps/api/tests/unit/operation/test_conductor.py b/apps/api/tests/unit/operation/test_conductor.py index b7060075855..082f18502e1 100644 --- a/apps/api/tests/unit/operation/test_conductor.py +++ b/apps/api/tests/unit/operation/test_conductor.py @@ -80,7 +80,9 @@ from cora.operation.conductor import ( ActionContext, ActionStep, + CaptureStep, CheckStep, + ComputeStep, Conductor, ConductorFailure, ConductorResult, @@ -2159,6 +2161,500 @@ async def test_conduct_complete_rejection_still_reports_what_was_left_set() -> N assert result.actuation_kind is ActuationKind.PHYSICAL +# --- closing steps ------------------------------------------------------- +# +# Pins the disposition table from [[project_conduct_closing_steps_design]]: +# closing runs whenever the main list reaches a Completed-bound success or +# a step failure (including a raised exception, the literal TomoScan +# shape) -- and runs BEFORE the corresponding complete_procedure / +# abort_procedure call, since closing's own journal writes need the +# Procedure still Running. Skipped on Held, an acquisition halt, and +# cancellation. A SUBSEQUENT complete_procedure rejection does not undo a +# closing walk that already ran. + + +@pytest.mark.unit +async def test_conduct_success_runs_closing_steps_and_merges_substrate_writes() -> None: + port, _ = _routed_port("2bma:main", "2bma:closing") + appender = _FakeAppendStep() + conductor = _conductor_full_lifecycle( + port, + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=_FakeLifecycleHandler(), + ids=[uuid4() for _ in range(4)], + ) + + result = await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:main", value=1.0),), + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.succeeded is True + assert result.closing_failures == () + assert dict(result.substrate_writes) == {"2bma:main": 1.0, "2bma:closing": 0.0} + + +@pytest.mark.unit +async def test_conduct_execute_failure_aborts_then_still_runs_closing_steps() -> None: + """A halted main list still runs closing after the best-effort abort -- + the shutter-open case this feature exists for.""" + port, _ = _routed_port("2bma:closing") # "2bma:missing" is NOT connected + appender = _FakeAppendStep() + abort = _FakeLifecycleHandler() + conductor = _conductor_full_lifecycle( + port, + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=abort, + ids=[uuid4() for _ in range(4)], + ) + + result = await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:missing", value=1.0),), + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.succeeded is False + assert len(abort.calls) == 1 + assert result.closing_failures == () + assert dict(result.substrate_writes) == {"2bma:closing": 0.0} + + +@pytest.mark.unit +async def test_conduct_closing_step_failure_is_isolated_and_does_not_change_succeeded() -> None: + """One closing step failing records it and the NEXT closing step still + runs; a closing failure never changes `succeeded` (that bit reports the + main steps only).""" + port, _ = _routed_port("2bma:main", "2bma:closing_ok") # "2bma:closing_bad" unconnected + appender = _FakeAppendStep() + conductor = _conductor_full_lifecycle( + port, + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=_FakeLifecycleHandler(), + ids=[uuid4() for _ in range(6)], + ) + + result = await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:main", value=1.0),), + closing_steps=( + SetpointStep(address="2bma:closing_bad", value=0.0), + SetpointStep(address="2bma:closing_ok", value=0.0), + ), + ) + + assert result.succeeded is True # main steps succeeded; closing failure is separate + assert len(result.closing_failures) == 1 + assert result.closing_failures[0].target == "2bma:closing_bad" + # The SECOND closing step still ran despite the first failing (isolated). + assert dict(result.substrate_writes) == {"2bma:main": 1.0, "2bma:closing_ok": 0.0} + + +@pytest.mark.unit +async def test_conduct_closing_step_that_raises_is_caught_not_propagated() -> None: + """`_run_closing` NEVER raises, even against a bug that would otherwise + propagate out of `_dispatch` (mirrors `_run_action`'s own non-port + exceptions propagating in the MAIN list): a raising closing step becomes + a recorded failure, and conduct() returns normally instead of raising.""" + + async def buggy(_ctx: ActionContext) -> Mapping[str, Any]: + raise RuntimeError("closing bug") + + registry = InMemoryActionRegistry({"buggy": buggy}) + port, _ = _routed_port("2bma:main") + appender = _FakeAppendStep() + conductor = _conductor_full_lifecycle( + port, + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=_FakeLifecycleHandler(), + ids=[uuid4() for _ in range(6)], + registry=registry, + ) + + result = await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:main", value=1.0),), + closing_steps=(ActionStep(name="buggy"),), + ) + + assert result.succeeded is True + assert len(result.closing_failures) == 1 + assert result.closing_failures[0].error_class == "RuntimeError" + assert result.closing_failures[0].target == "buggy" + + +class _RaisingPort: + """Every `read`/`write` raises a non-`_CONTROL_ERRORS` exception, so + `_run_setpoint`/`_run_check`/`_run_capture`'s own `except _CONTROL_ERRORS` + does not catch it -- it propagates to `_run_closing`'s classification + fallback (`_closing_step_kind`/`_closing_step_target`).""" + + async def read(self, address: str) -> Measurement: + raise RuntimeError("port read bug") + + async def write(self, *_args: Any, **_kwargs: Any) -> None: + raise RuntimeError("port write bug") + + def subscribe(self, address: str) -> AsyncIterator[Measurement]: # pragma: no cover # unused + raise NotImplementedError + + +@pytest.mark.unit +async def test_conduct_closing_setpoint_that_raises_classifies_as_setpoint() -> None: + """`_closing_step_kind`'s SetpointStep branch: a closing SetpointStep + whose write raises a non-port exception is still recorded, tagged + `source_kind="setpoint"`, `target=
`.""" + appender = _FakeAppendStep() + conductor = _conductor_full_lifecycle( + _RaisingPort(), + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=_FakeLifecycleHandler(), + ids=[uuid4() for _ in range(6)], + ) + + result = await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(), + closing_steps=(SetpointStep(address="2bma:closing", value=1.0),), + ) + + assert result.succeeded is True + assert len(result.closing_failures) == 1 + assert result.closing_failures[0].source_kind == "setpoint" + assert result.closing_failures[0].target == "2bma:closing" + assert result.closing_failures[0].error_class == "RuntimeError" + + +@pytest.mark.unit +async def test_conduct_closing_capture_that_raises_classifies_as_capture() -> None: + """`_closing_step_kind`'s CaptureStep branch.""" + appender = _FakeAppendStep() + conductor = _conductor_full_lifecycle( + _RaisingPort(), + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=_FakeLifecycleHandler(), + ids=[uuid4() for _ in range(6)], + ) + + result = await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(), + closing_steps=(CaptureStep(address="2bma:closing", capture_name="x"),), + ) + + assert result.succeeded is True + assert len(result.closing_failures) == 1 + assert result.closing_failures[0].source_kind == "capture" + assert result.closing_failures[0].target == "2bma:closing" + + +@pytest.mark.unit +async def test_conduct_closing_check_that_raises_classifies_as_check() -> None: + """`_closing_step_kind`'s default arm (CheckStep, the only Step kind not + named by an earlier `isinstance` branch).""" + appender = _FakeAppendStep() + conductor = _conductor_full_lifecycle( + _RaisingPort(), + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=_FakeLifecycleHandler(), + ids=[uuid4() for _ in range(6)], + ) + + result = await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(), + closing_steps=(CheckStep(address="2bma:closing", criterion=EqualsCriterion(expected=1)),), + ) + + assert result.succeeded is True + assert len(result.closing_failures) == 1 + assert result.closing_failures[0].source_kind == "check" + assert result.closing_failures[0].target == "2bma:closing" + + +@pytest.mark.unit +async def test_conduct_closing_compute_that_raises_classifies_as_compute() -> None: + """`_closing_step_kind`/`_closing_step_target`'s ComputeStep branches. + No `compute_port` wired, so `_run_compute` raises immediately -- the + loudest, simplest way to exercise this arm without a real substrate.""" + port = InMemoryControlPort() + appender = _FakeAppendStep() + conductor = _conductor_full_lifecycle( + port, + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=_FakeLifecycleHandler(), + ids=[uuid4() for _ in range(6)], + ) + + result = await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(), + closing_steps=(ComputeStep(command=("tomopy", "recon"), input_uris=(), output_uri=None),), + ) + + assert result.succeeded is True + assert len(result.closing_failures) == 1 + assert result.closing_failures[0].source_kind == "compute" + assert result.closing_failures[0].target == "tomopy recon" + + +@pytest.mark.unit +async def test_conduct_cancellation_does_not_run_closing_steps() -> None: + """A cancellation re-raises before any closing walk is attempted.""" + + class _CountingCancellingPort: + write_calls = 0 + + async def read(self, _address: str) -> Measurement: # pragma: no cover # unused + raise NotImplementedError + + async def write(self, *_args: Any, **_kwargs: Any) -> None: + type(self).write_calls += 1 + raise asyncio.CancelledError + + def subscribe(self, _address: str) -> AsyncIterator[Measurement]: # pragma: no cover + raise NotImplementedError + + _CountingCancellingPort.write_calls = 0 + appender = _FakeAppendStep() + conductor = Conductor( + control_port=_CountingCancellingPort(), # type: ignore[arg-type] + append_step=appender, + clock=FakeClock(_FIXED_NOW), + id_generator=_SequenceIdGenerator([]), + start_procedure=_FakeLifecycleHandler(), + complete_procedure=_FakeLifecycleHandler(), + abort_procedure=_FakeLifecycleHandler(), + ) + with pytest.raises(asyncio.CancelledError): + await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="any", value=1.0),), + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + # Exactly ONE write attempt (the main step's, which raised): closing was + # never dispatched, so it never reached the port at all. + assert _CountingCancellingPort.write_calls == 1 + + +@pytest.mark.unit +async def test_conduct_raised_exception_still_runs_closing_before_reraising() -> None: + """The literal TomoScan shape: an unhandled exception from an action body + must not skip closing. Best-effort abort fires first (closing runs on a + real terminal), closing runs, then the ORIGINAL exception propagates + unchanged.""" + + async def buggy(_ctx: ActionContext) -> Mapping[str, Any]: + raise RuntimeError("oops") + + registry = InMemoryActionRegistry({"buggy": buggy}) + port, inner = _routed_port("2bma:closing") + appender = _FakeAppendStep() + abort = _FakeLifecycleHandler() + conductor = _conductor_full_lifecycle( + port, + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=abort, + ids=[uuid4() for _ in range(4)], + registry=registry, + ) + + with pytest.raises(RuntimeError, match="oops"): + await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(ActionStep(name="buggy"),), + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + assert len(abort.calls) == 1 + assert "unhandled exception" in abort.calls[0].command.reason + landed = await inner.read("2bma:closing") + assert landed.value == 0.0 + + +@pytest.mark.unit +async def test_conduct_complete_rejected_still_reports_closing_that_already_ran() -> None: + """Closing runs BEFORE complete_procedure is even attempted (its journal + writes need the Procedure still Running -- append_activities accepts + entries only in Running). So a SUBSEQUENT complete_procedure rejection + does not undo the physical fact that closing already happened; the + ledger and any closing_failures still surface.""" + port, _ = _routed_port("2bma:shutter", "2bma:closing") + appender = _FakeAppendStep() + conductor = _conductor_full_lifecycle( + port, + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(raises=RuntimeError("complete rejected")), + abort=_FakeLifecycleHandler(), + ids=[uuid4() for _ in range(4)], + ) + + result = await conductor.conduct( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:shutter", value=1),), + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.succeeded is False + assert result.failure is not None + assert result.failure.target == "complete" + assert result.closing_failures == () + assert dict(result.substrate_writes) == {"2bma:shutter": 1, "2bma:closing": 0.0} + + +@pytest.mark.unit +async def test_conduct_or_hold_raised_exception_still_runs_closing_before_reraising() -> None: + """Mirrors conduct()'s twin: an unhandled exception from an action body + must not skip closing. Best-effort abort fires after closing (closing's + journal writes need the Procedure still Running), then the ORIGINAL + exception propagates unchanged.""" + + async def buggy(_ctx: ActionContext) -> Mapping[str, Any]: + raise RuntimeError("oops") + + registry = InMemoryActionRegistry({"buggy": buggy}) + port, inner = _routed_port("2bma:closing") + appender = _FakeAppendStep() + abort = _FakeLifecycleHandler() + conductor = Conductor( + control_port=port, + append_step=appender, + clock=FakeClock(_FIXED_NOW), + id_generator=_SequenceIdGenerator([uuid4() for _ in range(4)]), + action_registry=registry, + start_procedure=_FakeLifecycleHandler(), + complete_procedure=_FakeLifecycleHandler(), + abort_procedure=abort, + hold_procedure=_FakeLifecycleHandler(), + ) + + with pytest.raises(RuntimeError, match="oops"): + await conductor.conduct_or_hold( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(ActionStep(name="buggy"),), + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + assert len(abort.calls) == 1 + assert "unhandled exception" in abort.calls[0].command.reason + landed = await inner.read("2bma:closing") + assert landed.value == 0.0 + + +@pytest.mark.unit +async def test_conduct_or_hold_held_procedure_does_not_run_closing_steps() -> None: + """A hold is a pause: a later conduct_from resumes against the state the + main steps established, so closing (which would tear that down) must + not run.""" + port, inner = _routed_port("2bma:shutter", "2bma:closing") + inner.set_reading("2bma:rot:rbv", _good_reading(12.5)) + appender = _FakeAppendStep() + hold = _FakeLifecycleHandler() + conductor = _conductor_hold_lifecycle( + port, + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(), + abort=_FakeLifecycleHandler(), + hold=hold, + ids=[uuid4() for _ in range(4)], + ) + + result = await conductor.conduct_or_hold( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=( + SetpointStep(address="2bma:shutter", value=1), + CheckStep(address="2bma:rot:rbv", criterion=EqualsCriterion(expected=45.0)), + ), + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.held is True + assert len(hold.calls) == 1 + assert dict(result.substrate_writes) == {"2bma:shutter": 1} # NOT "2bma:closing" + + +@pytest.mark.unit +async def test_conduct_or_hold_non_recoverable_failure_aborts_then_runs_closing_steps() -> None: + """An acquisition (action) failure is non-recoverable: abort, not hold -- + and closing runs on that abort, exactly like conduct().""" + registry = InMemoryActionRegistry({}) # "collect" is unregistered -> UnknownActionError + port, inner = _routed_port("2bma:closing") + appender = _FakeAppendStep() + abort = _FakeLifecycleHandler() + hold = _FakeLifecycleHandler() + conductor = Conductor( + control_port=port, + append_step=appender, + clock=FakeClock(_FIXED_NOW), + id_generator=_SequenceIdGenerator([uuid4() for _ in range(4)]), + action_registry=registry, + start_procedure=_FakeLifecycleHandler(), + complete_procedure=_FakeLifecycleHandler(), + abort_procedure=abort, + hold_procedure=hold, + ) + + result = await conductor.conduct_or_hold( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(ActionStep(name="collect"),), + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.held is False + assert result.succeeded is False + assert len(abort.calls) == 1 + assert hold.calls == [] + landed = await inner.read("2bma:closing") + assert landed.value == 0.0 + + @pytest.mark.unit async def test_conduct_or_hold_complete_rejection_still_reports_what_was_left_set() -> None: """conduct_or_hold's complete arm carries the same obligation as conduct's.""" @@ -2188,6 +2684,37 @@ async def test_conduct_or_hold_complete_rejection_still_reports_what_was_left_se assert result.actuation_kind is ActuationKind.PHYSICAL +@pytest.mark.unit +async def test_conduct_or_hold_complete_rejected_still_reports_closing_that_already_ran() -> None: + """Same correction as conduct()'s twin: closing runs before the + complete_procedure attempt, so a subsequent rejection does not undo it.""" + port, _ = _routed_port("2bma:shutter", "2bma:closing") + appender = _FakeAppendStep() + conductor = _conductor_hold_lifecycle( + port, + appender, + start=_FakeLifecycleHandler(), + complete=_FakeLifecycleHandler(raises=RuntimeError("complete rejected")), + abort=_FakeLifecycleHandler(), + hold=_FakeLifecycleHandler(), + ids=[uuid4() for _ in range(4)], + ) + + result = await conductor.conduct_or_hold( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:shutter", value=1),), + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.succeeded is False + assert result.failure is not None + assert result.failure.target == "complete" + assert result.closing_failures == () + assert dict(result.substrate_writes) == {"2bma:shutter": 1, "2bma:closing": 0.0} + + @pytest.mark.unit async def test_conduct_or_hold_held_procedure_reports_what_was_left_set() -> None: """A Held Procedure is parked mid-flight awaiting an operator decision, @@ -2253,3 +2780,185 @@ async def test_conduct_from_complete_rejection_reports_the_merged_kind_and_ledge assert result.failure.target == "complete" assert dict(result.substrate_writes) == {"2bma:shutter": 1} assert result.actuation_kind is ActuationKind.PHYSICAL + + +@pytest.mark.unit +async def test_conduct_from_complete_rejected_still_reports_closing_that_already_ran() -> None: + """Same correction as conduct()'s twin: closing runs before the + complete_procedure attempt, so a subsequent rejection does not undo it.""" + port, _ = _routed_port("2bma:shutter", "2bma:closing") + appender = _FakeAppendStep() + conductor = Conductor( + control_port=port, + append_step=appender, + clock=FakeClock(_FIXED_NOW), + id_generator=_SequenceIdGenerator([uuid4() for _ in range(4)]), + resume_procedure=_FakeLifecycleHandler(), + complete_procedure=_FakeLifecycleHandler(raises=RuntimeError("complete rejected")), + abort_procedure=_FakeLifecycleHandler(), + ) + + result = await conductor.conduct_from( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:shutter", value=1),), + boundary=0, + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.succeeded is False + assert result.failure is not None + assert result.failure.target == "complete" + assert result.closing_failures == () + assert dict(result.substrate_writes) == {"2bma:shutter": 1, "2bma:closing": 0.0} + + +@pytest.mark.unit +async def test_conduct_from_raised_exception_still_runs_closing_before_reraising() -> None: + """Mirrors conduct()'s twin for the resume path: an unhandled exception + from a replayed step still runs closing (for its recording side-effect), + then the ORIGINAL exception propagates. Unlike conduct()/conduct_or_hold, + NO best-effort abort precedes it: a mid-replay raised exception leaves + the Procedure Running, the same posture as an acquisition halt, for the + operator to reconcile. An ActionStep can't trigger this here -- + execute_from halts-for-operator on one rather than running it -- so the + raise comes from the replayed SetpointStep's write instead, on a port + that raises for every address except the closing one.""" + + class _RaisingExceptForClosing: + def __init__(self, safe: InMemoryControlPort, safe_address: str) -> None: + self._safe = safe + self._safe_address = safe_address + + async def read(self, address: str) -> Measurement: + if address == self._safe_address: + return await self._safe.read(address) + raise RuntimeError("port read bug") + + async def write(self, address: str, value: Any, **kwargs: Any) -> None: + if address == self._safe_address: + await self._safe.write(address, value, **kwargs) + return + raise RuntimeError("oops") + + def subscribe(self, address: str) -> AsyncIterator[Measurement]: + raise NotImplementedError + + safe = InMemoryControlPort() + safe.simulate_connect("2bma:closing") + appender = _FakeAppendStep() + abort = _FakeLifecycleHandler() + conductor = Conductor( + control_port=_RaisingExceptForClosing(safe, "2bma:closing"), + append_step=appender, + clock=FakeClock(_FIXED_NOW), + id_generator=_SequenceIdGenerator([uuid4() for _ in range(4)]), + resume_procedure=_FakeLifecycleHandler(), + complete_procedure=_FakeLifecycleHandler(), + abort_procedure=abort, + ) + + with pytest.raises(RuntimeError, match="oops"): + await conductor.conduct_from( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:main", value=1.0),), + boundary=0, + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + assert abort.calls == [] + landed = await safe.read("2bma:closing") + assert landed.value == 0.0 + + +@pytest.mark.unit +async def test_conduct_from_clean_tail_completes_with_closing_steps_applied() -> None: + port, _ = _routed_port("2bma:shutter", "2bma:closing") + appender = _FakeAppendStep() + conductor = Conductor( + control_port=port, + append_step=appender, + clock=FakeClock(_FIXED_NOW), + id_generator=_SequenceIdGenerator([uuid4() for _ in range(4)]), + resume_procedure=_FakeLifecycleHandler(), + complete_procedure=_FakeLifecycleHandler(), + abort_procedure=_FakeLifecycleHandler(), + ) + + result = await conductor.conduct_from( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:shutter", value=1),), + boundary=0, + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.succeeded is True + assert result.closing_failures == () + assert dict(result.substrate_writes) == {"2bma:shutter": 1, "2bma:closing": 0.0} + + +@pytest.mark.unit +async def test_conduct_from_acquisition_halt_does_not_run_closing_steps() -> None: + """The replay tail hits an ActionStep: halt-for-operator, no transition. + Closing must not foreclose the redo-fresh-vs-reseed decision.""" + port, _ = _routed_port("2bma:closing") + appender = _FakeAppendStep() + abort = _FakeLifecycleHandler() + conductor = Conductor( + control_port=port, + append_step=appender, + clock=FakeClock(_FIXED_NOW), + id_generator=_SequenceIdGenerator([uuid4() for _ in range(4)]), + resume_procedure=_FakeLifecycleHandler(), + complete_procedure=_FakeLifecycleHandler(), + abort_procedure=abort, + ) + + result = await conductor.conduct_from( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(ActionStep(name="collect"),), + boundary=0, + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.succeeded is False + assert result.failure is not None + assert result.failure.error_class == "AcquisitionResumeRequiresOperator" + assert abort.calls == [] + assert dict(result.substrate_writes) == {} # closing never touched the port + + +@pytest.mark.unit +async def test_conduct_from_genuine_failure_aborts_then_runs_closing_steps() -> None: + port, _ = _routed_port("2bma:closing") # "2bma:missing" is NOT connected + appender = _FakeAppendStep() + abort = _FakeLifecycleHandler() + conductor = Conductor( + control_port=port, + append_step=appender, + clock=FakeClock(_FIXED_NOW), + id_generator=_SequenceIdGenerator([uuid4() for _ in range(4)]), + resume_procedure=_FakeLifecycleHandler(), + complete_procedure=_FakeLifecycleHandler(), + abort_procedure=abort, + ) + + result = await conductor.conduct_from( + procedure_id=uuid4(), + principal_id=uuid4(), + correlation_id=uuid4(), + steps=(SetpointStep(address="2bma:missing", value=1.0),), + boundary=0, + closing_steps=(SetpointStep(address="2bma:closing", value=0.0),), + ) + + assert result.succeeded is False + assert len(abort.calls) == 1 + assert result.closing_failures == () + assert dict(result.substrate_writes) == {"2bma:closing": 0.0} diff --git a/apps/api/tests/unit/operation/test_record_resolved_steps.py b/apps/api/tests/unit/operation/test_record_resolved_steps.py index 58ff49f549b..be96fcb9b99 100644 --- a/apps/api/tests/unit/operation/test_record_resolved_steps.py +++ b/apps/api/tests/unit/operation/test_record_resolved_steps.py @@ -73,6 +73,36 @@ def test_decide_records_resolved_steps_for_defined_procedure() -> None: assert event.occurred_at == _NOW +@pytest.mark.unit +def test_decide_records_resolved_closing_steps_in_a_separate_field() -> None: + """resolved_closing_steps is NOT flattened into resolved_steps: a flat pin + would let a conduct_from boundary land inside the closing region.""" + _, registered = _registered() + state = fold([registered]) + steps = ({"kind": "setpoint", "address": "2bma:rot", "value": 1.0, "verify": False},) + closing = ({"kind": "setpoint", "address": "2bma:shutter", "value": 0.0, "verify": False},) + + events = decide_resolved_steps_recorded(state, steps, now=_NOW, resolved_closing_steps=closing) + + assert len(events) == 1 + event = events[0] + assert event.resolved_steps == steps + assert event.resolved_closing_steps == closing + assert event.step_count == 2 # 1 main + 1 closing, mirrors RecipeExpansionRecorded + + +@pytest.mark.unit +def test_decide_defaults_resolved_closing_steps_to_empty() -> None: + _, registered = _registered() + state = fold([registered]) + steps = ({"kind": "setpoint", "address": "a", "value": 1.0, "verify": False},) + + event = decide_resolved_steps_recorded(state, steps, now=_NOW)[0] + + assert event.resolved_closing_steps == () + assert event.step_count == 1 + + @pytest.mark.unit def test_decide_records_nothing_when_state_is_none() -> None: steps = ({"kind": "setpoint", "address": "a", "value": 1.0, "verify": False},) diff --git a/apps/api/tests/unit/operation/test_register_procedure_from_recipe_decider.py b/apps/api/tests/unit/operation/test_register_procedure_from_recipe_decider.py index 8c2f277f581..acd59b7af03 100644 --- a/apps/api/tests/unit/operation/test_register_procedure_from_recipe_decider.py +++ b/apps/api/tests/unit/operation/test_register_procedure_from_recipe_decider.py @@ -1,5 +1,6 @@ """Unit tests for the `register_procedure_from_recipe` slice's pure decider.""" +import hashlib from collections.abc import Mapping from datetime import UTC, datetime from typing import Any @@ -7,7 +8,7 @@ import pytest -from cora.operation._recipe_expansion import expand +from cora.operation._recipe_expansion import canonical_json_bytes, expand, steps_to_wire from cora.operation.adapters.in_memory_recipe_expander import ( InMemoryRecipeExpander, ) @@ -111,6 +112,167 @@ def test_decide_emits_registered_plus_recipe_expansion_recorded() -> None: assert prov.step_count == 1 +@pytest.mark.unit +def test_decide_hashes_closing_steps_into_the_same_steps_hash_pin() -> None: + """A recipe whose only difference is closing_steps must hash differently + (one combined pin, no aliasing) -- proves closing_steps actually reaches + the hash, not just step_count.""" + cap = _capability() + plain = _recipe(cap.id) + with_closing = Recipe( + id=plain.id, + name=plain.name, + capability_id=plain.capability_id, + steps=plain.steps, + status=plain.status, + closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + ) + + plain_events = decide( + state=None, + command=_cmd(plain.id), + recipe=plain, + capability=cap, + expansion_port=InMemoryRecipeExpander(), + now=_NOW, + new_id=uuid4(), + ) + closing_events = decide( + state=None, + command=_cmd(with_closing.id), + recipe=with_closing, + capability=cap, + expansion_port=InMemoryRecipeExpander(), + now=_NOW, + new_id=uuid4(), + ) + plain_prov = plain_events[1] + closing_prov = closing_events[1] + assert isinstance(plain_prov, RecipeExpansionRecorded) + assert isinstance(closing_prov, RecipeExpansionRecorded) + assert plain_prov.steps_hash != closing_prov.steps_hash + assert closing_prov.step_count == 2 # 1 main + 1 closing + + +@pytest.mark.unit +def test_decide_empty_closing_steps_hashes_identically_to_no_closing_field() -> None: + """The migration-claim precedent applied to closing_steps: an empty + closing list must hash EXACTLY like a recipe that never had the field, + so no existing pinned expansion is invalidated by this feature.""" + cap = _capability() + recipe = _recipe(cap.id) + assert recipe.closing_steps == () + + events = decide( + state=None, + command=_cmd(recipe.id), + recipe=recipe, + capability=cap, + expansion_port=InMemoryRecipeExpander(), + now=_NOW, + new_id=uuid4(), + ) + prov = events[1] + assert isinstance(prov, RecipeExpansionRecorded) + expanded = InMemoryRecipeExpander().expand(recipe.steps, {}) + legacy_hash = hashlib.sha256(canonical_json_bytes(steps_to_wire(expanded))).hexdigest() + assert prov.steps_hash == legacy_hash + + +@pytest.mark.unit +def test_decide_raises_overflow_when_combined_count_exceeds_cap() -> None: + """Overflow must range over steps + closing_steps combined, not steps alone.""" + cap = _capability() + recipe = Recipe( + id=uuid4(), + name=RecipeName("R"), + capability_id=cap.id, + steps=(RecipeSetpointStep(address="dev:x", value=1.0),), + closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + ) + + class _FakeSplitOverflowPort: + version = "v1" + + def expand( + self, + steps: tuple[RecipeStep, ...], + bindings: Mapping[str, Any], + ) -> tuple[Step, ...]: + _ = bindings + from cora.operation.conductor import SetpointStep + + # Main list alone is well under cap; closing list alone pushes + # the COMBINED count over it. + first = steps[0] if steps else None + is_main = ( + len(steps) == 1 + and isinstance(first, RecipeSetpointStep) + and first.address == "dev:x" + ) + size = 5 if is_main else 10_000 + return tuple(SetpointStep(address=f"x:{i}", value=i) for i in range(size)) + + with pytest.raises(RecipeExpansionOverflowError) as exc: + decide( + state=None, + command=_cmd(recipe.id), + recipe=recipe, + capability=cap, + expansion_port=_FakeSplitOverflowPort(), # type: ignore[arg-type] + now=_NOW, + new_id=uuid4(), + ) + assert exc.value.step_count == 10_005 + assert exc.value.cap == 10_000 + + +@pytest.mark.unit +def test_decide_raises_determinism_error_on_closing_steps_divergence() -> None: + """A port that diverges only on the closing-list expansion must still + trip the determinism gate, not just a main-list divergence.""" + cap = _capability() + recipe = Recipe( + id=uuid4(), + name=RecipeName("R"), + capability_id=cap.id, + steps=(RecipeSetpointStep(address="dev:x", value=1.0),), + closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + ) + + class _FakeNonDeterministicClosingPort: + version = "v1" + _calls = 0 + + def expand( + self, + steps: tuple[RecipeStep, ...], + bindings: Mapping[str, Any], + ) -> tuple[Step, ...]: + from cora.operation.conductor import SetpointStep + + first = steps[0] if steps else None + if ( + len(steps) == 1 + and isinstance(first, RecipeSetpointStep) + and first.address == "dev:shutter" + ): + self._calls += 1 + return (SetpointStep(address="dev:shutter", value=float(self._calls)),) + return expand(steps, bindings) + + with pytest.raises(RecipeExpansionDeterminismError): + decide( + state=None, + command=_cmd(recipe.id), + recipe=recipe, + capability=cap, + expansion_port=_FakeNonDeterministicClosingPort(), # type: ignore[arg-type] + now=_NOW, + new_id=uuid4(), + ) + + @pytest.mark.unit def test_decide_records_patience_cap_on_event() -> None: cap = _capability() @@ -252,7 +414,12 @@ def expand( steps: tuple[RecipeStep, ...], bindings: Mapping[str, Any], ) -> tuple[Step, ...]: - _ = steps, bindings + # Overflow is isolated to the main list; an empty input (the + # closing_steps call, since big_recipe has none) must expand to + # empty, or the combined count double-counts a fixed-size fake. + _ = bindings + if not steps: + return () from cora.operation.conductor import SetpointStep return tuple(SetpointStep(address=f"x:{i}", value=i) for i in range(10_001)) diff --git a/apps/api/tests/unit/recipe/test_define_recipe_handler.py b/apps/api/tests/unit/recipe/test_define_recipe_handler.py index a8533fff57c..974c1998029 100644 --- a/apps/api/tests/unit/recipe/test_define_recipe_handler.py +++ b/apps/api/tests/unit/recipe/test_define_recipe_handler.py @@ -192,6 +192,52 @@ async def test_handler_raises_binding_unknown_parameter_when_schema_missing_key( ) +@pytest.mark.unit +async def test_handler_raises_binding_unknown_parameter_for_a_bad_ref_in_closing_steps() -> None: + """A bad BindingRef in closing_steps must be caught too, not just in steps + -- proving the handler validates the CONCATENATED walk, not steps alone.""" + _, deps = await _build_seeded_deps( + parameters_schema={"type": "object", "properties": {"angle": {"type": "number"}}} + ) + handler = define_recipe.bind(deps) + + with pytest.raises(RecipeBindingReferencesUnknownParameterError): + await handler( + DefineRecipe( + name="R", + capability_id=_CAPABILITY_ID, + steps=(RecipeSetpointStep(address="dev:x", value=BindingRef("angle")),), + closing_steps=( + RecipeSetpointStep(address="dev:shutter", value=BindingRef("enrgy")), + ), + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + +@pytest.mark.unit +async def test_handler_persists_closing_steps_on_the_recipe() -> None: + store, deps = await _build_seeded_deps() + handler = define_recipe.bind(deps) + + recipe_id = await handler( + DefineRecipe( + name="R", + capability_id=_CAPABILITY_ID, + steps=(RecipeSetpointStep(address="dev:x", value=1.0),), + closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + assert recipe_id == _NEW_ID + recipe_events, _ = await store.load("Recipe", _NEW_ID) + assert recipe_events[0].payload["closing"]["steps"] == [ + {"kind": "setpoint", "address": "dev:shutter", "value": 0.0, "verify": False} + ] + + @pytest.mark.unit async def test_handler_raises_empty_recipe_steps_when_command_steps_empty() -> None: _, deps = await _build_seeded_deps() diff --git a/apps/api/tests/unit/recipe/test_recipe_events.py b/apps/api/tests/unit/recipe/test_recipe_events.py index 900393a2ada..dc50107f940 100644 --- a/apps/api/tests/unit/recipe/test_recipe_events.py +++ b/apps/api/tests/unit/recipe/test_recipe_events.py @@ -85,6 +85,37 @@ def test_to_payload_recipe_versioned_serializes_version_tag_and_steps() -> None: assert "capability_id" not in payload # capability_id preserved on state, not in payload +@pytest.mark.unit +def test_to_payload_recipe_defined_serializes_closing_steps_under_closing_key() -> None: + rid, cid = uuid4(), uuid4() + defn = RecipeDefined( + recipe_id=rid, + name="R", + capability_id=cid, + steps=_steps(), + occurred_at=_NOW, + closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + ) + payload = to_payload(defn) + assert payload["closing"] == { + "steps": [{"kind": "setpoint", "address": "dev:shutter", "value": 0.0, "verify": False}] + } + + +@pytest.mark.unit +def test_to_payload_recipe_versioned_serializes_closing_steps_under_closing_key() -> None: + rid = uuid4() + ver = RecipeVersioned( + recipe_id=rid, + version_tag="v2", + steps=_steps(), + occurred_at=_NOW, + closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + ) + payload = to_payload(ver) + assert payload["closing"]["steps"] + + @pytest.mark.unit def test_to_payload_recipe_deprecated_serializes_replaced_by_or_none() -> None: rid, succ = uuid4(), uuid4() @@ -114,6 +145,40 @@ def test_from_stored_round_trips_recipe_versioned() -> None: assert rebuilt == original +@pytest.mark.unit +def test_from_stored_round_trips_recipe_defined_with_non_empty_closing_steps() -> None: + """Equality alone proves nothing about a field left at its default (`()`); + this pins a NON-default closing_steps surviving the round trip.""" + rid, cid = uuid4(), uuid4() + original = RecipeDefined( + recipe_id=rid, + name="R", + capability_id=cid, + steps=_steps(), + occurred_at=_NOW, + closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + ) + stored = _stored("RecipeDefined", to_payload(original)) + rebuilt = from_stored(stored) + assert rebuilt == original + assert isinstance(rebuilt, RecipeDefined) + assert rebuilt.closing_steps == (RecipeSetpointStep(address="dev:shutter", value=0.0),) + + +@pytest.mark.unit +def test_from_stored_recipe_defined_tolerates_a_pre_feature_payload_missing_closing_key() -> None: + """A RecipeDefined stored before closing steps existed has no "closing" key + at all; from_stored must default it to empty, not raise KeyError.""" + rid, cid = uuid4(), uuid4() + original = _make_defined(rid, cid) + payload = to_payload(original) + del payload["closing"] + stored = _stored("RecipeDefined", payload) + rebuilt = from_stored(stored) + assert isinstance(rebuilt, RecipeDefined) + assert rebuilt.closing_steps == () + + @pytest.mark.unit def test_from_stored_round_trips_recipe_deprecated_with_replacement() -> None: rid, succ = uuid4(), uuid4() diff --git a/apps/api/tests/unit/recipe/test_recipe_evolver.py b/apps/api/tests/unit/recipe/test_recipe_evolver.py index 3fc9431da07..2cf715121f4 100644 --- a/apps/api/tests/unit/recipe/test_recipe_evolver.py +++ b/apps/api/tests/unit/recipe/test_recipe_evolver.py @@ -82,6 +82,51 @@ def test_recipe_versioned_replaces_steps_wholesale_and_preserves_identity() -> N assert state2.name == state.name +@pytest.mark.unit +def test_recipe_defined_folds_closing_steps() -> None: + event = _defined(closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),)) + state = evolve(None, event) + assert state.closing_steps == (RecipeSetpointStep(address="dev:shutter", value=0.0),) + + +@pytest.mark.unit +def test_recipe_defined_defaults_closing_steps_to_empty() -> None: + state = evolve(None, _defined()) + assert state.closing_steps == () + + +@pytest.mark.unit +def test_recipe_versioned_replaces_closing_steps_wholesale() -> None: + rid = uuid4() + state = evolve( + None, + _defined(recipe_id=rid, closing_steps=(RecipeSetpointStep(address="dev:a", value=1.0),)), + ) + new_closing = (RecipeSetpointStep(address="dev:b", value=2.0),) + state2 = evolve( + state, + RecipeVersioned( + recipe_id=rid, + version_tag="v1", + steps=state.steps, + occurred_at=_NOW, + closing_steps=new_closing, + ), + ) + assert state2.closing_steps == new_closing + + +@pytest.mark.unit +def test_recipe_deprecated_preserves_closing_steps_for_audit() -> None: + rid = uuid4() + state = evolve( + None, + _defined(recipe_id=rid, closing_steps=(RecipeSetpointStep(address="dev:a", value=1.0),)), + ) + state2 = evolve(state, RecipeDeprecated(reason="Superseded", recipe_id=rid, occurred_at=_NOW)) + assert state2.closing_steps == state.closing_steps + + @pytest.mark.unit def test_recipe_deprecated_preserves_steps_and_capability_id_for_audit() -> None: rid, cid, succ = uuid4(), uuid4(), uuid4() diff --git a/apps/api/tests/unit/recipe/test_version_recipe_handler.py b/apps/api/tests/unit/recipe/test_version_recipe_handler.py index 555018904c8..05130614b2e 100644 --- a/apps/api/tests/unit/recipe/test_version_recipe_handler.py +++ b/apps/api/tests/unit/recipe/test_version_recipe_handler.py @@ -86,6 +86,28 @@ async def test_handler_appends_recipe_versioned_event() -> None: assert events[1].payload["version_tag"] == "v1" +@pytest.mark.unit +async def test_handler_persists_closing_steps_on_the_new_version() -> None: + store, deps = await _build_seeded_deps() + handler = version_recipe.bind(deps) + + await handler( + VersionRecipe( + recipe_id=_RECIPE_ID, + version_tag="v1", + steps=(RecipeSetpointStep(address="dev:x", value=2.0),), + closing_steps=(RecipeSetpointStep(address="dev:shutter", value=0.0),), + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) + + events, _ = await store.load("Recipe", _RECIPE_ID) + assert events[1].payload["closing"]["steps"] == [ + {"kind": "setpoint", "address": "dev:shutter", "value": 0.0, "verify": False} + ] + + @pytest.mark.unit async def test_handler_raises_unauthorized_on_deny() -> None: _, deps = await _build_seeded_deps(deny=True) @@ -180,3 +202,19 @@ async def test_handler_re_validates_binding_refs_against_capability_schema() -> principal_id=_PRINCIPAL_ID, correlation_id=_CORRELATION_ID, ) + + # A bad ref in closing_steps alone (main steps are valid) must ALSO + # raise -- proving the handler validates the CONCATENATED walk. + with pytest.raises(RecipeBindingReferencesUnknownParameterError): + await handler( + VersionRecipe( + recipe_id=_RECIPE_ID, + version_tag="v2", + steps=(RecipeSetpointStep(address="dev:x", value=BindingRef("angle")),), + closing_steps=( + RecipeSetpointStep(address="dev:shutter", value=BindingRef("enrgy")), + ), + ), + principal_id=_PRINCIPAL_ID, + correlation_id=_CORRELATION_ID, + ) diff --git a/apps/api/tools/gen_record_dispositions.py b/apps/api/tools/gen_record_dispositions.py index 8a0005b2efa..37ecfe56e1f 100644 --- a/apps/api/tools/gen_record_dispositions.py +++ b/apps/api/tools/gen_record_dispositions.py @@ -77,6 +77,11 @@ ("SealOnlineKeyRotated", "facility_code"): "facility_id", ("SealRepublishingStarted", "facility_code"): "facility_id", ("SealRepublishingCompleted", "facility_code"): "facility_id", + # `to_payload` writes the closing-steps list under the short wire key + # "closing" (matching the operation-BC vocabulary the design settled + # on: "closing steps", not "teardown"), not the dataclass field name. + ("RecipeDefined", "closing_steps"): "closing", + ("RecipeVersioned", "closing_steps"): "closing", } # A field's TYPE-DRIVEN disposition is occasionally the wrong call for diff --git a/docs/deployments/2-bm/recipes.md b/docs/deployments/2-bm/recipes.md index dde33e483ed..32b8d94124d 100644 --- a/docs/deployments/2-bm/recipes.md +++ b/docs/deployments/2-bm/recipes.md @@ -54,8 +54,13 @@ Both momentary-command questions are now answered by the same run: writing `1` t | 1 | setpoint | `S02BM-PSS:SBS:OpenEPICSC` (StationShutter, open command) | `1` (verify) | | 2 | check | `S02BM-PSS:SBS:BeamBlockingM` (StationShutter, status read-back) | `== "OFF"` (open, inverted polarity) | | 3 | action | `collect` | `{ detector: "2bmSP1:", repetitions: <>, dwell: <> }` | -| 4 | setpoint | `S02BM-PSS:SBS:CloseEPICSC` (StationShutter, close command) | `1` (verify, return to safe state) | -| 5 | check | `S02BM-PSS:SBS:BeamBlockingM` (StationShutter, status read-back) | `== "ON"` (closed) | + +**Closing steps** (see [glossary](../../reference/glossary.md#recipe-ladder)): walked once `steps` above reaches a real terminal (Completed or Aborted), so the shutter closes even if the capture halts partway through instead of only on a clean run. + +| # | Step | Address | Value / params | +| --- | --- | --- | --- | +| 1 | setpoint | `S02BM-PSS:SBS:CloseEPICSC` (StationShutter, close command) | `1` (verify, return to safe state) | +| 2 | check | `S02BM-PSS:SBS:BeamBlockingM` (StationShutter, status read-back) | `== "ON"` (closed) | **Precondition:** the sample is out of the beam path. This is an operator assertion, not a CORA setpoint (CORA does not drive the sample out automatically). diff --git a/docs/reference/glossary.md b/docs/reference/glossary.md index a0686e215d1..99d62f80572 100644 --- a/docs/reference/glossary.md +++ b/docs/reference/glossary.md @@ -98,6 +98,7 @@ Watch-only (not adopted as a glossary term, see [Deferred](../stack/deferred.md) - **Run.** An execution of a Plan. FSM `RunStatus`: `Running`, `Held`, `Completed`, `Aborted`, `Stopped`, `Truncated`. - **Recipe.** *(Recipe BC)* The executable body: a deployment-bound, ordered tuple of templated steps referencing one Capability by `capability_id`, which expands into the flat Conductor step list once an operator supplies parameter bindings. Sits BESIDE the ladder rather than on a rung of it: Method stays the technique-class contract and Plan stays the Asset-bound binding, while Recipe carries the steps and iterates faster than the Capability above it. Steps change only through `version_recipe` (they replace wholesale, and the prior version stays readable), so a Recipe body cannot be silently mutated. - **RecipeStep.** *(Recipe BC, closed union)* Five templated arms, each the twin of one Conductor `Step`: `RecipeSetpointStep`, `RecipeActionStep`, `RecipeCheckStep`, `RecipeCaptureStep`, `RecipeComputeStep`. A step position that holds a value the Recipe does not itself carry uses one of four typed sentinels, which differ in when they resolve and who produces the value: `BindingRef` (expansion time, from the operator's frozen bindings), `CaptureRef` (execute time, from a reading an earlier step deposited), `SteeringRef` (execute time, from the coordinate the decide loop seeds each pass), `OutputRef` (execute time, from an artifact an earlier compute step produced). Substitution is typed, never textual `${var}`. The union carries no branch, loop, or fork arm: control flow lives outside the body, in the conduct verb (`conduct_until_converged`, `conduct_until_advised`) or in a `Held` transition an operator resumes. See the [Recipe module](../architecture/modules/recipe/index.md#step-kinds-and-value-substitution). +- **Closing steps.** *(Recipe BC / Conductor)* A Recipe's second, optional step list (`closing_steps`, same five `RecipeStep` arms as `steps`). The Conductor walks it once `steps` reaches a real terminal, Completed or Aborted, never on a `Held` pause, an acquisition halt, or cancellation. Each closing step is isolated: one failing does not stop the rest of the walk or flip the conduct's own `succeeded`, and every failure lands in `closing_failures` on the result. Exists so a Recipe can return equipment to a safe state (close a shutter, verify closed) even when the main step list halted early. `dark_field` already ends safe (its shutter-close is step 1 of `steps`); `flat_field` moves its own shutter-close + verify into `closing_steps` so a halt mid-capture still closes the shutter. - **Logbook.** Append-only narrative log on a Run or Decision. Used for OTel `gen_ai.*` reasoning entries on Decisions. ## Agents