fix(engine): gate a reflexive "when you do" on the optional action actually happening

crates/engine/src/game/effects/mod.rs

@@ -12442,19 +12442,36 @@ pub(crate) fn evaluate_condition(
             .is_some_and(|f| f.flipper == ability.controller && f.result == *result),
         // CR 603.12: A reflexive triggered ability ("when you do") triggers
         // "based on whether the trigger event or events occurred earlier during
-        // the resolution" of the parent. For a cost-payment parent
-        // (`Effect::PayCost`), an unpayable or declined cost is NOT a trigger
-        // event occurrence, so the reflexive sub-ability must NOT fire — the
-        // `PayCost` and mandatory-discard handlers signal this via
-        // `cost_payment_failed_flag` (mirrors `IfYouDo` above). An accepted
-        // "you may discard a card" with an empty hand did not discard a card,
-        // so it cannot create the reflexive trigger. Other non-cost parents
-        // (e.g. `BecomeCopy` reflexives) remain unconditional.
+        // the resolution" of the parent. Two independent ways the parent event
+        // can fail to occur, each read through the authority that owns it:
+        //
+        // 1. An OPTIONAL parent whose action was never performed — declined, or
+        //    never offered because it was impossible (CR 608.2d;
+        //    `optional_effect_is_infeasible`, e.g. "you may remove an oil
+        //    counter" with no oil counters). `optional_effect_performed` is the
+        //    engine's single record of "the player took the optional action",
+        //    and it is exactly what the sibling connector `IfYouDo`
+        //    (`EffectOutcome { OptionalEffectPerformed }`, above) reads. The two
+        //    connectors ask the same question about the same parent, so they
+        //    must consult the same authority — "when you do" is not a weaker
+        //    "if you do". A MANDATORY parent carries no such record (the flag
+        //    stays false because no choice was ever offered), so the gate is
+        //    scoped to `ability.optional` and mandatory reflexives
+        //    (`BecomeCopy`, `RollDie`) remain unconditional.
+        // 2. An accepted parent whose payment then failed: unpayable cost, or an
+        //    accepted "you may discard a card" with an empty hand. The `PayCost`
+        //    and mandatory-discard handlers signal this via
+        //    `cost_payment_failed_flag`. This is NOT subsumed by (1) — the
+        //    optional action WAS taken, it is the payment underneath that did
+        //    not happen — so both gates are load-bearing.
         AbilityCondition::WhenYouDo => {
-            !(matches!(
+            let optional_action_not_taken =
+                ability.optional && !ability.context.optional_effect_performed;
+            let payment_failed = matches!(
                 ability.effect,
                 Effect::PayCost { .. } | Effect::Discard { .. } | Effect::DiscardCard { .. }
-            ) && state.cost_payment_failed_flag)
+            ) && state.cost_payment_failed_flag;
+            !optional_action_not_taken && !payment_failed
         }
         // CR 601.2a + CR 707.10: "was cast (from [zone])" — check cast origin.
         // `zone: None` = cast from any origin; a copy or put-into-play object has
@@ -18222,6 +18239,52 @@ mod tests {
         );
     }
 
+    /// CR 603.12 + CR 608.2d: the reflexive connector "when you do" asks the
+    /// same question about the same parent as its sibling "if you do", so it
+    /// must read the same authority — `optional_effect_performed`. An optional
+    /// parent whose action was declined or never offered (because it was
+    /// impossible: "you may remove an oil counter" with no oil counters,
+    /// Atraxa's Skitterfang) did not produce the trigger event.
+    ///
+    /// The mandatory axis is the discriminator that keeps this from
+    /// over-suppressing: a parent with no "may" carries no performed-record at
+    /// all, so gating on the bare flag would silence every mandatory reflexive
+    /// (`RollDie`, `BecomeCopy`). All three rows below run against the same
+    /// `RemoveCounter` effect so the only thing varying is optionality and the
+    /// record — the effect type cannot be what carries the answer.
+    #[test]
+    fn when_you_do_reads_the_optional_performed_record_not_the_effect_type() {
+        let state = GameState::new_two_player(42);
+        let remove_counter = || Effect::RemoveCounter {
+            counter_type: Some(CounterType::Generic("oil".to_string())),
+            count: QuantityExpr::Fixed { value: 1 },
+            target: TargetFilter::SelfRef,
+        };
+        let parent = |optional: bool, performed: bool| {
+            let mut ability =
+                ResolvedAbility::new(remove_counter(), vec![], ObjectId(100), PlayerId(0));
+            ability.optional = optional;
+            ability.context.optional_effect_performed = performed;
+            ability
+        };
+
+        assert!(
+            !evaluate_condition(&AbilityCondition::WhenYouDo, &state, &parent(true, false)),
+            "an optional parent whose action was never performed produced no \
+             trigger event, so the reflexive must not fire (CR 603.12)"
+        );
+        assert!(
+            evaluate_condition(&AbilityCondition::WhenYouDo, &state, &parent(true, true)),
+            "an optional parent whose action WAS performed must still fire its \
+             reflexive — the gate must not suppress the working card"
+        );
+        assert!(
+            evaluate_condition(&AbilityCondition::WhenYouDo, &state, &parent(false, false)),
+            "a MANDATORY parent carries no performed-record; gating on the bare \
+             flag would silence every mandatory reflexive"
+        );
+    }
+
     #[test]
     fn chain_depth_exceeds_limit_returns_error() {
         let mut state = GameState::new_two_player(42);

crates/engine/src/types/ability.rs

@@ -20707,12 +20707,13 @@ pub enum AbilityCondition {
     /// process") and any cross-sentence flip-result gate.
     CoinFlipOutcome { result: CoinFlipResult },
     /// CR 603.12: "When you do" — reflexive trigger that fires based on whether the
-    /// parent's trigger event actually occurred. For a non-cost parent (e.g. a
-    /// `BecomeCopy` reflexive or a copy/exile replacement sub-ability) the "do"
-    /// always occurred, so this is unconditionally true. For a cost-payment parent
-    /// (`Effect::PayCost`), an unpayable or declined cost is not an occurrence, so
-    /// the reflexive sub-ability is skipped — `evaluate_condition` gates on
-    /// `cost_payment_failed_flag` for that case (mirrors `IfYouDo`).
+    /// parent's trigger event actually occurred. A mandatory non-cost parent (e.g.
+    /// a `BecomeCopy` reflexive or a copy/exile replacement sub-ability) always
+    /// occurred, while an optional non-cost parent must actually be performed.
+    /// For a cost-payment parent (`Effect::PayCost`), an unpayable or declined cost
+    /// is not an occurrence, so the reflexive sub-ability is skipped —
+    /// `evaluate_condition` gates on `cost_payment_failed_flag` for that case
+    /// (mirrors `IfYouDo`).
     WhenYouDo,
     /// CR 601.2a + CR 707.10: "if [this spell] was cast from [zone]" — sub_ability
     /// executes only if the spell was cast. `zone: None` = cast from any origin;

crates/engine/tests/integration/issue_1328_inti.rs

@@ -62,6 +62,19 @@ fn inti_attack_trigger_ast_has_reflexive_counter_after_optional_discard() {
     );
 }
 
+fn hand_count(
+    runner: &engine::game::scenario::GameRunner,
+    player: engine::types::PlayerId,
+) -> usize {
+    runner
+        .state()
+        .players
+        .iter()
+        .find(|p| p.id == player)
+        .map(|p| p.hand.len())
+        .expect("player exists")
+}
+
 fn p1p1(runner: &engine::game::scenario::GameRunner, id: ObjectId) -> u32 {
     runner
         .state()
@@ -202,3 +215,83 @@ fn inti_reflexive_counter_after_interactive_discard_choice() {
         "Inti must also grant trample to the chosen attacker"
     );
 }
+
+/// PR #7414 review (CodeRabbit): the deferred-continuation carrier loses the
+/// parent's `optional`, so the question was whether a DECLINED optional parent
+/// can run its reflexive after a suspension.
+///
+/// It cannot, and this row pins why: declining ends the chain before anything
+/// suspends. `DiscardChoice` — the suspension in this card — is only reached
+/// once the "you may" has been ACCEPTED, so there is no continuation to resume
+/// and the reflexive is never created. Two cards stay in hand, no counter, no
+/// trample.
+///
+/// The accept-side twin is `inti_reflexive_counter_after_interactive_discard_choice`
+/// above; together they cover both answers to the same prompt on the one card
+/// whose suspended path the resolver's own comment names.
+///
+/// Stated plainly: this row does NOT discriminate the PR #7414 gate — measured,
+/// it passes with that gate reverted too, because an explicitly declined
+/// optional never reaches the condition at all. It pins the reachability
+/// argument (decline ⇒ no suspension ⇒ no resumed carrier), which is what makes
+/// the carrier's missing `optional` harmless.
+#[test]
+fn inti_declined_discard_suspends_nothing_and_fires_no_reflexive() {
+    let mut scenario = GameScenario::new();
+    scenario.at_phase(Phase::PreCombatMain);
+    scenario.add_creature_from_oracle(P0, "Inti, Seneschal of the Sun", 2, 2, INTI_ATTACK_ABILITY);
+    let attacker = scenario.add_creature(P0, "Attacker", 2, 2).id();
+    scenario.add_card_to_hand(P0, "Discard A");
+    scenario.add_card_to_hand(P0, "Discard B");
+
+    let mut runner = scenario.build();
+    runner.pass_both_players();
+    runner
+        .act(GameAction::DeclareAttackers {
+            attacks: vec![(attacker, AttackTarget::Player(P1))],
+            bands: vec![],
+        })
+        .expect("declare attackers");
+    runner.pass_both_players();
+
+    let hand_before = hand_count(&runner, P0);
+    runner
+        .act(GameAction::DecideOptionalEffect { accept: false })
+        .expect("declining the optional discard must be allowed");
+
+    assert!(
+        !matches!(runner.state().waiting_for, WaitingFor::DiscardChoice { .. }),
+        "declining must not suspend into a discard choice, got {:?}",
+        runner.state().waiting_for
+    );
+    assert!(
+        !matches!(
+            runner.state().waiting_for,
+            WaitingFor::TriggerTargetSelection { .. }
+        ),
+        "CR 603.12: nothing was discarded, so the reflexive must not target, got {:?}",
+        runner.state().waiting_for
+    );
+
+    runner.advance_until_stack_empty();
+
+    assert_eq!(
+        hand_count(&runner, P0),
+        hand_before,
+        "declining must not discard a card"
+    );
+    assert_eq!(
+        p1p1(&runner, attacker),
+        0,
+        "a declined discard puts no +1/+1 counter on the attacker"
+    );
+    assert!(
+        !runner
+            .state()
+            .objects
+            .get(&attacker)
+            .expect("attacker remains on the battlefield")
+            .has_keyword(&Keyword::Trample),
+        "a declined discard grants no trample"
+    );
+}

crates/engine/tests/integration/main.rs

@@ -962,6 +962,7 @@ mod serpent_society_ward_poison_cost;
 mod serras_emissary_chosen_card_type_protection;
 mod shorten_efficacy;
 mod sin_spiras_punishment_repeat;
+mod skitterfang_reflexive_without_counter;
 mod skullwinder_chosen_opponent;
 mod slaughter_the_strong_total_power_4380;
 mod slitherwisp_flash_spell_cast_trigger;