fix(parser): guard an existing player_scope + pin the reason in #6965's fail-closed tests

crates/engine/src/parser/oracle_effect/mod.rs

@@ -18653,11 +18653,11 @@ fn rewrite_recipient_on_link(def: &mut AbilityDefinition, filter: &TargetFilter)
 /// `player_scope_from_parent_target_subject`), reused here rather than
 /// duplicated.
 ///
-/// Total and FAIL-CLOSED: only a recipient an existing `PlayerFilter` can name
-/// is bound. A recipient naming a TARGETED player ("target opponent") or an
-/// object ("that creature") returns `false`, so the caller falls through to
-/// `Effect::Unimplemented` instead of silently letting the printed controller
-/// act in someone else's place.
+/// Total and FAIL-CLOSED on BOTH sides of the binding. A recipient naming a
+/// TARGETED player ("target opponent") or an object ("that creature") returns
+/// `false`; so does a body that already carries its own scope. Either way the
+/// caller falls through to `Effect::Unimplemented` instead of silently letting
+/// the printed controller act in someone else's place.
 fn bind_recipient_without_recipient_slot(
     def: &mut AbilityDefinition,
     filter: &TargetFilter,
@@ -18668,6 +18668,15 @@ fn bind_recipient_without_recipient_slot(
     if matches!(filter, TargetFilter::OriginalController) {
         return true;
     }
+    // The body may already carry a printed iteration scope of its own. Both
+    // halves are rewritten from clones of ONE parsed body, so an unguarded
+    // stamp would replace that fan-out with a single recipient — and with a
+    // DIFFERENT one on each half, since each half binds its own conjunct. The
+    // printed per-player iteration would be silently lost. Refuse instead: an
+    // honest gap beats resolving for the wrong set of players.
+    if def.player_scope.is_some() {
+        return false;
+    }
     let Some(scope) = distribution_recipient_player_scope(filter) else {
         return false;
     };

crates/engine/src/parser/oracle_effect/tests.rs

@@ -559,11 +559,13 @@ fn possessive_actor_compound_subject_distributes_a_recipient_bearing_body() {
     );
 }
 
-/// CR 109.4 + CR 608.2d: the recipient-less binding channel generalizes past the
+/// CR 608.2d: the recipient-less binding channel generalizes past the
 /// possessive axis. Infernal Offering's "You and that player each sacrifice a
 /// creature" has no `TargetFilter` recipient slot on `Effect::Sacrifice` either,
 /// and its second conjunct is the opponent a preceding "Choose an opponent."
-/// picked — so the second half binds `player_scope: ChosenPlayer`.
+/// picked — a choice announced while applying the effect, not an object whose
+/// controller is being read — so the second half binds
+/// `player_scope: ChosenPlayer`.
 #[test]
 fn recipient_less_body_binds_a_chosen_player_conjunct_by_scope() {
     let parsed = parse_oracle_text(
@@ -602,23 +604,55 @@ fn recipient_less_body_binds_a_chosen_player_conjunct_by_scope() {
     );
 }
 
-/// CR 109.4: FAIL-CLOSED contract for the recipient-less binding channel. No
-/// `PlayerFilter` can name a TARGETED player, so "you and target opponent each
+/// CR 115.1: FAIL-CLOSED contract for the recipient-less binding channel. The
+/// second conjunct is a TARGETED player — declared as the spell goes on the
+/// stack — and no `PlayerFilter` names one, so "you and target opponent each
 /// flip a coin" (Mana Clash) / "… each secretly choose 1, 2, or 3"
-/// (Expert-Level Safe) must stay an honest `Unimplemented` — binding the body to
+/// (Expert-Level Safe) must stay an honest `Unimplemented`. Binding the body to
 /// `PlayerFilter::Opponent` would make EVERY opponent act in a multiplayer game,
 /// and leaving it unbound would make the caster act twice.
 #[test]
 fn recipient_less_body_with_a_targeted_player_conjunct_fails_closed() {
+    // Positive reach guard: the SAME grammar with a NAMEABLE conjunct must still
+    // parse. Without it, every assertion below would also hold if the
+    // compound-subject distributor had stopped running altogether — a dead path
+    // fails closed on everything, including cases it should bind.
+    //
+    // "that player" needs its antecedent: the binding comes from the preceding
+    // "Choose an opponent.", so the guard has to carry that sentence. A bare
+    // "You and that player each …" is itself an unbound subject, which is why
+    // this guard uses the full Infernal Offering text.
+    let reachable = parse_oracle_text(
+        "Choose an opponent. You and that player each sacrifice a creature.",
+        "Infernal Offering",
+        &[],
+        &["Sorcery".to_string()],
+        &[],
+    );
+    let bound_half = reachable
+        .abilities
+        .first()
+        .and_then(|ability| ability.sub_ability.as_deref())
+        .expect("reach-guard: the caster half must exist after the Choose");
+    assert!(
+        matches!(&*bound_half.effect, Effect::Sacrifice { .. }),
+        "reach-guard: the compound-subject distributor must still bind a nameable \
+         conjunct, else the fail-closed assertions below are vacuous, got {:#?}",
+        bound_half.effect
+    );
+
     for text in [
         "You and target opponent each flip a coin.",
         "You and target opponent each secretly choose 1, 2, or 3.",
     ] {
         let ability = parse_effect_chain(text, AbilityKind::Spell);
-        assert!(
-            matches!(&*ability.effect, Effect::Unimplemented { .. }),
-            "{text:?} must fail closed, got {:#?}",
-            ability.effect
+        let Effect::Unimplemented { name, .. } = &*ability.effect else {
+            panic!("{text:?} must fail closed, got {:#?}", ability.effect);
+        };
+        assert_eq!(
+            name, "unbound_subject",
+            "{text:?} must fail closed AT THE SUBJECT — any other gap name means the \
+             clause died earlier and this case stopped covering the targeted-player path"
         );
         assert_eq!(
             ability.player_scope, None,

crates/engine/src/parser/oracle_ir/ast.rs

@@ -193,11 +193,15 @@ pub(crate) struct SubjectPhraseAst {
     /// BOARD-WIDE effect — the grant landed on every permanent, lands and
     /// artifacts included, while coverage still reported the card as supported.
     /// Encoding the unbound state in the type makes that fail-open
-    /// unrepresentable: every consumer must say what it does with `None`, and
-    /// the one consumer that actually reads this field
-    /// (`lower_subject_predicate_ast`'s `ImperativeFallback` arm, the only
-    /// predicate kind that applies the subject filter) fails closed to
-    /// `Effect::unimplemented`. Same shape, same reason, as
+    /// unrepresentable: every consumer must say what it does with `None`.
+    /// `lower_subject_predicate_ast`'s `ImperativeFallback` arm — the only
+    /// predicate kind that applies the subject filter — is the only consumer
+    /// that treats `None` as a coverage GAP, failing closed to
+    /// `Effect::unimplemented`. The other readers
+    /// (`sync_subject_into_nested_shuffle_sub`, `inject_subject_target`) reach
+    /// it through `target.or(affected)` and treat `None` as "nothing to
+    /// rebind", returning early. `None` is therefore reachable in all three —
+    /// do not assume otherwise when editing them. Same shape, same reason, as
     /// [`EntersUnderSpec::UnboundAnaphor`].
     pub(crate) affected: Option<TargetFilter>,
     pub(crate) target: Option<TargetFilter>,

crates/engine/tests/integration/wand_of_orcus_compound_subject_6965.rs

@@ -21,13 +21,16 @@
 //!
 //! CR 611.2c: one continuous effect naming several subjects determines the set
 //! each part applies to independently — i.e. the UNION of the named subjects.
-//! CR 301.5f: an Equipment attaches to a creature.
+//! CR 301.5a: an Equipment is attached to a creature, which is then the
+//! "equipped creature". CR 301.5f: an ability referring to the "equipped
+//! creature" means whatever creature the permanent is attached to.
 //! CR 702.2b: deathtouch. CR 702.11b: hexproof.
 
 use engine::game::combat::AttackTarget;
 use engine::game::game_object::AttachTarget;
 use engine::game::layers::evaluate_layers;
 use engine::game::scenario::{GameRunner, GameScenario, P0, P1};
+use engine::types::ability::Effect;
 use engine::types::card_type::CoreType;
 use engine::types::identifiers::ObjectId;
 use engine::types::keywords::Keyword;
@@ -76,8 +79,9 @@ fn wand_of_orcus_unbindable_subject_grants_nothing_board_wide() {
 
     let mut runner = scenario.build();
 
-    // CR 301.5f: make the Wand a real Equipment attached to `host`, so its
-    // "equipped creature attacks" trigger has a subject to fire on.
+    // CR 301.5a: attach the Wand to `host` so it is a real Equipment with an
+    // equipped creature. CR 301.5f: that is what its "equipped creature
+    // attacks" trigger resolves against, so the trigger has a subject to fire on.
     {
         let obj = runner.state_mut().objects.get_mut(&wand).unwrap();
         obj.card_types.core_types = vec![CoreType::Artifact];
@@ -104,6 +108,38 @@ fn wand_of_orcus_unbindable_subject_grants_nothing_board_wide() {
         "the attack trigger must be on the stack, or nothing below is exercised"
     );
 
+    // ...and the trigger must carry the SPECIFIC gap this test is about. Stack
+    // presence alone proves only that a trigger was created: a regression that
+    // dropped the execute effect entirely would also grant no deathtouch and
+    // leave every assertion below green. Pin the reason, not just the silence.
+    {
+        let wand_obj = runner.state().objects.get(&wand).unwrap();
+        let gap = wand_obj
+            .trigger_definitions
+            .iter_unchecked()
+            .filter_map(|entry| entry.definition.execute.as_ref())
+            .find_map(|exec| match exec.effect.as_ref() {
+                Effect::Unimplemented { name, description } => Some((name, description)),
+                _ => None,
+            })
+            .expect(
+                "the attack trigger's execute chain must be an Unimplemented gap — if it \
+                 parsed, or vanished, the deathtouch assertions below prove nothing",
+            );
+        assert_eq!(
+            gap.0, "unbound_subject",
+            "the gap must name the SUBJECT as the unbound part; another name means the \
+             clause failed elsewhere and this test stopped covering the fail-closed path"
+        );
+        assert!(
+            gap.1
+                .as_deref()
+                .is_some_and(|text| text.contains("Zombies you control")),
+            "reach-guard: the gap must quote the unbindable conjunct, got {:?}",
+            gap.1
+        );
+    }
+
     runner.advance_until_stack_empty();
     runner.state_mut().layers_dirty.mark_full();
     evaluate_layers(runner.state_mut());