fix(engine): loop-ring retention across forced windows + observer-negative rulings (combo-fb phases 0-2, chain 1)

crates/engine/src/analysis/decision_template.rs

@@ -212,6 +212,26 @@ pub struct ShortcutDecisionSchema {
     /// CR 732.1b: the proposed repeat mode. `UntilLethal` for a determinate CR 704.5a /
     /// CR 704.5c drain; `Fixed(n)` seeds the frontend count picker for an optional loop.
     pub iteration_count: IterationCount,
+    /// CR 732.2a: the largest number of repetitions this proposal may legally specify — the
+    /// minimum over every applicable CR 704 elimination bound and finite-pool bound, over
+    /// every LIVING player, aggregated per declarable victim, clamped to
+    /// `MAX_SHORTCUT_CYCLES`. `IterationCount` above is the *suggestion*; this is the
+    /// *bound*, and they are deliberately separate fields: a proposal that exceeds this
+    /// contains a conditional action (an in-proposal CR 704.5a / CR 704.5c / CR 104.3c /
+    /// CR 121.4 elimination would decide what happens next), which CR 732.2a forbids.
+    ///
+    /// The single count authority: the declared-count check in `game::engine` rejects a
+    /// `Fixed(n)` above it, and `game::interaction` publishes it as the count picker's
+    /// ceiling. Every offer built before the bounded-offer phase carries
+    /// `MAX_SHORTCUT_CYCLES`, so those checks are inert until a producer narrows it.
+    ///
+    /// DELIBERATELY NOT MIRRORED in `client/src/adapter/types.ts::ShortcutDecisionSchema`:
+    /// the frontend never reads the raw bound, it reads the already-clamped ceiling the
+    /// engine publishes as `InteractionShortcutCountSpec::Fixed { max }`. Mirroring it
+    /// would hand the display layer a second number it would have to reconcile — exactly
+    /// the derive-in-the-frontend the layer rule forbids.
+    #[serde(default = "default_max_iterations")]
+    pub max_iterations: u32,
     /// The open per-iteration decision-points needing pins. EMPTY for a choice-free drain.
     pub points: Vec<DecisionPoint>,
     /// CR 702.51a: total untapped creatures the controller may tap for convoke across every
@@ -221,13 +241,21 @@ pub struct ShortcutDecisionSchema {
     pub convoke_tappable_count: usize,
 }
 
+/// A schema deserialized from a pre-bound snapshot carries no CR 732.2a count bound. The
+/// forward-compatible default is the global safety limit, which is what every producer
+/// emitted before the field existed — so an old save round-trips byte-equivalently.
+fn default_max_iterations() -> u32 {
+    crate::game::engine::MAX_SHORTCUT_CYCLES
+}
+
 // CR 732.2a: `IterationCount` carries no `Default` and its `Fixed(u32)` is a tuple variant
 // (so a derived `#[default]` cannot apply) — hand-impl the forward-compat deser default the
 // `#[serde(default)]` on `WaitingFor::LoopShortcut.schema` needs.
 impl Default for ShortcutDecisionSchema {
     fn default() -> Self {
         Self {
             iteration_count: IterationCount::Fixed(0),
+            max_iterations: default_max_iterations(),
             points: Vec::new(),
             convoke_tappable_count: 0,
         }
@@ -861,6 +889,9 @@ mod tests {
     fn shortcut_decision_schema_round_trips_and_defaults() {
         let schema = ShortcutDecisionSchema {
             iteration_count: IterationCount::UntilLethal,
+            // A NARROWED CR 732.2a bound, deliberately not the default: a round-trip that
+            // carried the default would pass even if the field were dropped from the wire.
+            max_iterations: 17,
             points: vec![DecisionPoint {
                 slot: DecisionSlot {
                     source: all_copies(7),
@@ -879,16 +910,35 @@ mod tests {
             convoke_tappable_count: 2,
         };
         let json = serde_json::to_value(&schema).expect("serialize");
+        assert_eq!(
+            json["max_iterations"], 17,
+            "the CR 732.2a bound must reach the wire — a `#[serde(default)]` field that is \
+             never serialized would silently reset to the cap on every reload"
+        );
         let back: ShortcutDecisionSchema = serde_json::from_value(json).expect("deserialize");
         assert_eq!(back, schema);
         assert_eq!(
             ShortcutDecisionSchema::default(),
             ShortcutDecisionSchema {
                 iteration_count: IterationCount::Fixed(0),
+                max_iterations: crate::game::engine::MAX_SHORTCUT_CYCLES,
                 points: vec![],
                 convoke_tappable_count: 0,
             }
         );
+        // A pre-bound snapshot (no `max_iterations` key at all) must load at the cap, which
+        // is exactly what every producer emitted before the field existed.
+        let mut legacy = serde_json::to_value(ShortcutDecisionSchema::default()).unwrap();
+        legacy
+            .as_object_mut()
+            .expect("schema serializes as an object")
+            .remove("max_iterations");
+        assert_eq!(
+            serde_json::from_value::<ShortcutDecisionSchema>(legacy)
+                .expect("a pre-bound snapshot still deserializes")
+                .max_iterations,
+            crate::game::engine::MAX_SHORTCUT_CYCLES
+        );
     }
 
     /// Phase-1 `resolve`/gate tests don't consult `key`; give every template an empty

crates/engine/src/analysis/loop_check.rs

@@ -463,11 +463,51 @@ pub(crate) fn live_mandatory_loop_winner(
 /// (the one non-faller) is. A transient intra-cycle dip that recovers to a
 /// non-negative NET delta would still kill the winner via the CR 704.5a SBA at low
 /// absolute life before the extrapolated win — a net-delta check cannot see it.
-/// Per-resolution granularity IS SBA granularity here (CR 704.3 checks whenever a
-/// player would get priority, between resolutions), and consecutive ring frames are
-/// consecutive resolutions (a non-sampling beat clears the ring), so requiring
-/// `life[winner]` non-decreasing across the matched window (prior frame → every
-/// subsequent ring frame → the live state) is exactly right. Winner draw-from-empty
+/// Per-resolution granularity IS SBA granularity here, but NOT because ring frames are
+/// consecutive resolutions — they are not, and never were. The shipped CR 603.3b
+/// `OrderTriggers` exemption already retains the ring across a non-sampling beat (dump D
+/// measured 35 such beats in one drive), and `WaitingFor::is_forced_cascade_window`
+/// extends that to every forced pre-priority window (CR 603.3d / CR 603.5 + CR 608.2 /
+/// CR 903.9a / CR 704.5j / CR 310.10 / CR 703.1 + CR 117.3a). The invariant this guard
+/// actually needs is weaker and true:
+/// **every point at which CR 704.5a could fire is either sampled or clears the ring.**
+/// CR 704.3 fixes those points: SBAs are checked whenever a player would get priority,
+/// and every such point arrives as `WaitingFor::Priority`, which is deliberately not a
+/// forced-cascade window and therefore samples or clears. The retained windows are
+/// exempt for three DIFFERENT reasons, and the weaker invariant is what covers all
+/// three:
+/// the between-resolutions members (CR 603.3b / CR 603.3d / CR 903.9a / CR 704.5j /
+/// CR 310.10) sit inside the CR 704.3 fixpoint itself, where no life total moves; the
+/// MID-resolution member (`OptionalEffectChoice`, CR 603.5 + CR 608.2) is a pause in the
+/// middle of a resolution, where life absolutely can move — but CR 608.2 performs no SBA
+/// check mid-resolution, so a life change there is not a CR 704.5a point being skipped,
+/// it is a life change that the very next CR 704.3 check (a `Priority` window) observes;
+/// the TURN-BASED members (CR 703.1 + CR 117.3a — untap CR 502.3, declare attackers
+/// CR 508.1/508.1g, declare blockers CR 509.1, cleanup discard CR 514.1) precede the
+/// step's own grant of priority (CR 508.2 is the explicit case), and the DECLARATION
+/// itself moves no life: untapping, declaring, exerting/enlisting and discarding change
+/// no life, and anything that WOULD (an attack trigger) uses the stack and therefore
+/// resolves at an observed `Priority` beat.
+/// That is a claim about the declaration only, and the two life-moving neighbours it
+/// deliberately excludes are why the class is drawn where it is:
+/// * CR 508.1h / CR 509.1d put the declaration's COSTS in a separate sub-step
+///   ("Costs may include paying mana, tapping permanents, sacrificing permanents,
+///   discarding cards, and so on"), and a Phyrexian symbol in an attack or block tax is
+///   paid with 2 life (CR 107.4f) — measured in-code: `engine_combat::handle_pay_combat_tax`
+///   pays through `casting::pay_unless_cost`, which settles `life_payments` via
+///   `life_costs::pay_life_as_cost`. So declaring CAN move life, at
+///   `WaitingFor::CombatTaxPayment` — which is deliberately NOT a member and therefore
+///   clears the ring.
+/// * `AssignCombatDamage` / `AssignBlockerDamage` are likewise NOT members despite being
+///   turn-based (CR 510.1c / CR 510.1d): CR 510.2 deals the assigned damage with no
+///   intervening priority. That window-keyed exclusion is necessary but NOT sufficient,
+///   because the window opens only for a damage DIVISION choice — an unblocked attacker
+///   deals CR 510.2 damage with no window at all. The sufficient guard is event-keyed:
+///   `GameState::invalidate_loop_ring_on_unobserved_life_move`, called from
+///   `game::combat_damage::apply_combat_damage`.
+///
+/// So requiring `life[winner]` non-decreasing across the matched window (prior frame →
+/// every subsequent ring frame → the live state) is exactly right. Winner draw-from-empty
 /// is correctly unreachable (a non-faller never crosses a loss SBA).
 pub(crate) fn winner_life_never_dips(frames: &[&GameState], winner: PlayerId) -> bool {
     let mut prev: Option<i32> = None;
@@ -590,6 +630,13 @@ mod tests {
         PlayerId(n)
     }
 
+    // The CR 704.3 partition `winner_life_never_dips` rests on — `Priority` DISJOINT from
+    // the retained class, over both priority seats — is asserted by
+    // `types::game_state::forced_cascade_window_tests::forced_cascade_window_class`, which
+    // covers it strictly more completely (thirteen members and eight non-members, including both
+    // `Priority` seats). A second weaker row here would only be a place for the two to
+    // drift apart.
+
     fn battlefield_creature(state: &mut GameState, id: u64, controller: u8) -> ObjectId {
         let oid = ObjectId(id);
         let mut object = GameObject::new(