orchestrator: Let Platform::execute return a follow-up event

services/orchestrator/sm/src/lib.rs

@@ -5,7 +5,7 @@
 //!
 //! This is the pure-reducer core ported from `rot_reducer`. It describes side
 //! effects as [`Effect`] values rather than performing them; the surrounding
-//! OpenPRoT shell carries them out via a [`Platform`] impl. No concrete hardware
+//! OpenPRoT platform driver carries them out via a [`Platform`] impl. No concrete hardware
 //! appears here — the machine is generic over an opaque [`ComponentId`].
 //!
 //! See `docs/verification-model.md` and `docs/state-machine.md` in the
@@ -30,21 +30,20 @@ pub use model::*;
 // deployment. The board owns `N` (chain length), `E` (effect-buffer size) and
 // max_retry.
 
-/// Upper bound on how many events one settle can queue: the triggering outside
-/// event, at most one `Emit` follow-up (`RecoveryFailed`, emitted at most once
-/// before latching), and at most one injected `EffectFailed` (de-duplicated in
-/// `dispatch_with` — it is idempotent and terminal, so a second is never
-/// queued). Three total; `PENDING_CAP` keeps headroom above that so the pushes
-/// in `dispatch_with` can never overflow.
+/// Upper bound on events queued at once inside `dispatch_with`. The queue
+/// pops as it settles, so this bounds *in-flight* events, not a run's total
+/// length: one batch can queue at most one `Emit` follow-up plus one returned
+/// event per external effect. Executors that return an event for many effects
+/// of one batch can overflow this; overflow is fail-closed (see
+/// `dispatch_with`), never silent loss.
 const PENDING_CAP: usize = 8;
 
-/// Compile-time floor tying the queue capacity to that worst case, mirroring
-/// `Rot::EFFECT_CAP_OK` for the effect buffer. Evaluated at build time (an
-/// anonymous `const`), so an under-sized `PENDING_CAP` fails to compile rather
-/// than risking a runtime overflow.
+/// Compile-time floor: room for an `Emit` follow-up, a returned event, and
+/// the injected `EffectFailed`. Evaluated at build time (an anonymous
+/// `const`), so an under-sized `PENDING_CAP` fails to compile.
 const _: () = assert!(
     PENDING_CAP >= 3,
-    "PENDING_CAP must hold one outside event + one Emit follow-up + one EffectFailed",
+    "PENDING_CAP must hold an Emit follow-up + a returned event + EffectFailed",
 );
 
 /// Result of dispatching one event to a state (or its superstate).
@@ -155,7 +154,7 @@ struct ComponentStatus {
     retry: u8,
     /// Set while this component has been released from reset but has not yet
     /// reported its boot-progress signal ([`Event::ComponentReady`] for an
-    /// `Active` component, [`Event::Booted`] for a `Passive` one). The shell
+    /// `Active` component, [`Event::Booted`] for a `Passive` one). The platform driver
     /// arms a per-component watchdog on release; this bit is what a later
     /// [`Event::Timeout`] consults to tell a real boot failure from a stale or
     /// spurious timeout. Orthogonal to `lifecycle`: a gated component owes no
@@ -319,7 +318,7 @@ impl<const N: usize, const E: usize> Rot<N, E> {
     }
 
     /// Record that `id` has been released and now owes a boot-progress signal.
-    /// Paired with the `ReleaseReset` emitted at each release site: the shell
+    /// Paired with the `ReleaseReset` emitted at each release site: the platform driver
     /// arms its per-component boot watchdog there, and this arms ours. Also
     /// marks the component *live* (`released`), which a later re-entry to
     /// [`State::PreSupervision`] uses to quiesce it before re-verifying.
@@ -871,24 +870,39 @@ impl<const N: usize, const E: usize> Rot<N, E> {
     }
 }
 
-/// Signals that the shell could not carry out an [`Effect`]. The machine does
-/// not need the shell's error detail — **every** actuation failure is treated
+/// Signals that the platform driver could not carry out an [`Effect`]. The machine does
+/// not need the driver's error detail — **every** actuation failure is treated
 /// the same, fail-closed: the driver injects [`Event::EffectFailed`] and the
 /// machine latches to [`State::Locked`]. This blanket policy is deliberate and
 /// is what lets the failure signal stay a payload-less marker; a future design
 /// that needs per-effect recovery must add a *new*, descriptive event rather
-/// than widen this type. The shell logs the specifics on its side.
+/// than widen this type. The driver logs the specifics on its side.
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub struct EffectError;
 
 /// Outward connection to the platform. Carry out one effect, reporting
 /// [`EffectError`] if it could not be performed. Never called with
 /// [`Effect::Emit`] — the orchestrator consumes those internally.
 ///
+/// `Ok(Some(event))` feeds back what the effect produced synchronously (e.g.
+/// a verification verdict); the driver queues it and settles it in the same
+/// dispatch run. At most one event per effect. Synchronous results belong
+/// here, not in a driver-side queue — one feedback path keeps ordering honest.
+/// Never block in `execute`: results that arrive later (boot progress, timer
+/// expiry) are delivered as their own outside events via `dispatch`.
+///
+/// Failure stays on the error channel, never in a returned event: `Err` is
+/// checked between effects, so a failed actuation aborts the rest of the
+/// batch — a feedback event cannot do that.
+///
 /// Contract the reducer relies on:
 /// - **Honest, complete feedback.** The reducer's correctness rests entirely on
-///   the event stream the shell feeds back; dropping, reordering, or
+///   the event stream the driver feeds back; dropping, reordering, or
 ///   synthesizing events silently breaks the state machine's invariants.
+/// - **Returned events quiesce.** Every returned event reports a result the
+///   reducer consumes (its retry budgets bound re-verification cycles). An
+///   executor that manufactures an event for every effect keeps one dispatch
+///   run alive indefinitely.
 /// - **`AssertReset` holds, it does not pulse.** A reset must keep the component
 ///   quiesced and non-executing until its matching `ReleaseReset`. The reducer's
 ///   at-rest verification guarantee depends on this: it re-asserts reset on
@@ -898,10 +912,10 @@ pub struct EffectError;
 ///   component resume before verification and void that guarantee.
 /// - **A failed [`Effect::LatchLockdown`] is a hard fault.** Lockdown is the top
 ///   of the escalation ladder — the reducer has nothing stronger to emit and
-///   will *believe* it is `Locked`. The shell must treat that failure as
+///   will *believe* it is `Locked`. The driver must treat that failure as
 ///   terminal (halt/reset), not a recoverable error.
 pub trait Platform {
-    fn execute(&mut self, effect: Effect) -> Result<(), EffectError>;
+    fn execute(&mut self, effect: Effect) -> Result<Option<Event>, EffectError>;
 }
 
 /// A handle for a caller's own event loop. Owns the machine's storage
@@ -983,61 +997,77 @@ impl<const N: usize, const E: usize> Orchestrator<N, E> {
         }
     }
 
-    /// Handle one event all the way through — including any [`Effect::Emit`]
-    /// follow-ups — calling `on_effect` for each external effect in order.
+    /// Handle one event all the way through — every [`Effect::Emit`]
+    /// follow-up and every event the executors return — calling `on_effect`
+    /// for each external effect in order. One call runs to quiescence.
     ///
     /// If `on_effect` reports an [`EffectError`], the driver injects an
-    /// [`Event::EffectFailed`] into the same run, so a failed actuation is
-    /// handled fail-closed (the machine latches to [`State::Locked`]) rather
-    /// than silently ignored.
+    /// [`Event::EffectFailed`] at the *front* of the queue, so a failed
+    /// actuation is handled fail-closed: the latch settles next, and feedback
+    /// still queued behind it drains into [`State::Locked`] (discarded)
+    /// instead of actuating hardware after a failure. A pending-queue
+    /// overflow is handled the same way: losing a returned event would break
+    /// the honest-feedback contract, so the run latches instead.
     pub fn dispatch_with(
         &mut self,
         event: Event,
-        mut on_effect: impl FnMut(Effect) -> Result<(), EffectError>,
+        mut on_effect: impl FnMut(Effect) -> Result<Option<Event>, EffectError>,
     ) {
-        let mut pending: heapless::Vec<Event, PENDING_CAP> = heapless::Vec::new();
+        // Fail-closed latch: `EffectFailed` goes to the *front*, so it settles
+        // next and everything still queued drains into `Locked` (discarded)
+        // instead of actuating hardware after a failure. Prefer evicting the
+        // newest queued event over losing the latch itself.
+        fn latch(pending: &mut heapless::Deque<Event, PENDING_CAP>) {
+            if pending.is_full() {
+                pending.pop_back();
+            }
+            // Dead Err arm: the eviction above guarantees room.
+            let _ = pending.push_front(Event::EffectFailed);
+        }
+
+        let mut pending: heapless::Deque<Event, PENDING_CAP> = heapless::Deque::new();
         // Dead Err arm: `pending` is empty and `PENDING_CAP >= 3` (asserted at
         // build time), so the first push always fits.
-        let _ = pending.push(event);
-
-        let mut i = 0;
-        while i < pending.len() {
-            let ev = pending[i];
-            i += 1;
+        let _ = pending.push_back(event);
+        // `EffectFailed` is injected at most once: it is idempotent and
+        // terminal (drives to `Locked`, which discards everything after). The
+        // only external effect executed after it settles is `Locked`'s own
+        // entry, whose failure must not inject again.
+        let mut failed = false;
 
+        while let Some(ev) = pending.pop_front() {
             let mut buf = Sink::<E>::new();
             self.step(&ev, &mut buf);
 
             for &effect in buf.effects() {
-                match effect {
-                    Effect::Emit(internal) => {
-                        // Dead Err arm: the reducer emits at most one `Emit`
-                        // (`RecoveryFailed`) per settle, well within PENDING_CAP.
-                        let _ = pending.push(internal);
-                    }
-                    external => {
-                        if on_effect(external).is_err() {
-                            // Fail-closed AND fail-fast: abandon the rest of this
-                            // batch so no effect ordered after the failed one hits
-                            // hardware. `step` has already advanced the state as if
-                            // the whole batch applied, so actuating `k+1..` would
-                            // carry out effects for a transition we are about to
-                            // override by latching to `Locked`.
-                            //
-                            // Inject `EffectFailed` once: it is idempotent and
-                            // terminal (drives to `Locked`, which discards
-                            // everything after), so a second injection would be a
-                            // no-op. De-duping against this append-only queue —
-                            // which still holds the first `EffectFailed` as its own
-                            // marker — caps the queue at the worst case
-                            // `PENDING_CAP` is sized for. Dead Err arm: that bound
-                            // is below PENDING_CAP.
-                            if !pending.contains(&Event::EffectFailed) {
-                                let _ = pending.push(Event::EffectFailed);
+                let follow_up = match effect {
+                    // Internal: handle next, never forwarded to the platform.
+                    Effect::Emit(internal) => Some(internal),
+                    external => match on_effect(external) {
+                        Ok(follow_up) => follow_up,
+                        Err(_) => {
+                            // Fail-closed AND fail-fast: abandon the rest of
+                            // this batch. `step` has already advanced the
+                            // state as if the whole batch applied, and the
+                            // latch overrides that transition, so nothing
+                            // ordered after the failure may hit hardware.
+                            if !failed {
+                                failed = true;
+                                latch(&mut pending);
                             }
                             break;
                         }
-                    }
+                    },
+                };
+                if let Some(next) = follow_up
+                    && pending.push_back(next).is_err()
+                    && !failed
+                {
+                    // Queue full: `next` would be lost, breaking the
+                    // honest-feedback contract. Fail closed instead.
+                    failed = true;
+                    latch(&mut pending);
+                    break;
                 }
             }
         }

services/orchestrator/sm/src/model.rs

@@ -67,7 +67,7 @@ pub enum FailurePolicy {
 
 /// Opaque recovery-region key supplied by the board at chain-build time.
 /// Components sharing a `RegionId` are restored together: when any region
-/// member enters [`State::Recovering`], the shell resolves and restores the
+/// member enters [`State::Recovering`], the platform driver resolves and restores the
 /// whole region. The core treats this as an equality key only and never
 /// inspects membership itself.
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
@@ -182,7 +182,7 @@ pub enum PowerOnResult {
 /// Everything the outside world can tell the state machine.
 #[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum Event {
-    /// Power-on, carrying the shell's self-verification and provisioning result.
+    /// Power-on, carrying the platform driver's self-verification and provisioning result.
     PowerGood(PowerOnResult),
     /// The eRoT's signature + SVN check on this component passed.
     VerificationPassed(ComponentId),
@@ -215,7 +215,7 @@ pub enum Event {
     Restored(ComponentId),
     /// A required component's recovery was exhausted.
     RecoveryFailed,
-    /// The shell's boot-progress watchdog fired: `id` did not report its
+    /// The platform driver's boot-progress watchdog fired: `id` did not report its
     /// boot-progress signal ([`Event::ComponentReady`] for an `Active`
     /// component, [`Event::Booted`] for a `Passive` one) within its configured
     /// boot timeout. Treated as a verification failure — a component still
@@ -236,7 +236,7 @@ pub enum Event {
     /// when it executes [`Effect::ActivateUpdate`] and cancels it on
     /// [`Effect::CommitSvnFloor`].
     CommitTimeout,
-    /// The shell could not carry out an emitted [`Effect`]; fail-closed, it
+    /// The platform driver could not carry out an emitted [`Effect`]; fail-closed, it
     /// latches to [`State::Locked`] from any state. Injected by the driver when
     /// a [`Platform::execute`](crate::Platform::execute) call fails; never
     /// produced by a handler.