fix(phase-ai): repair AI land and mana decision-making across nine units

crates/engine/src/game/casting.rs

@@ -354,7 +354,15 @@ pub(crate) fn begin_variable_speed_payment(
 
 /// CR 107.3a + CR 118.3: X in an activation/additional cost is chosen as part
 /// of activating or casting, bounded by the resources available to pay fully.
-pub(crate) fn sacrifice_cost_bounds(count: u32, eligible_len: usize) -> (usize, usize) {
+///
+/// `pub` as the single authority for the `u32::MAX` X-sentinel encoding, so a
+/// cast-time cost gate in `phase-ai` reads the engine's own minimum rather than
+/// re-spelling the sentinel. That is a *prospective* single-authority argument,
+/// not a measured divergence: for every `count < u32::MAX` this returns
+/// `(n, n)`, which a hand-rolled `count as usize` matches exactly. It becomes
+/// load-bearing the day a third bounds shape ("up to N") is added, at which
+/// point a copy would desynchronize with no compile error.
+pub fn sacrifice_cost_bounds(count: u32, eligible_len: usize) -> (usize, usize) {
     if count == u32::MAX {
         (0, eligible_len)
     } else {
@@ -16385,7 +16393,16 @@ fn find_pay_life_cost(
 /// CR 118.3: Find permanents controlled by `player` matching `filter` on the battlefield.
 /// The source is eligible when it matches the printed filter; "another" is
 /// represented by `FilterProp::Another` and enforced by `matches_target_filter`.
-pub(super) fn find_eligible_sacrifice_targets(
+///
+/// The single authority for sacrifice-cost eligibility. Three conditions, and all
+/// three matter: controller (CR 701.21a — "a player can't sacrifice … something
+/// that's a permanent they don't control"), the `player_cant_sacrifice_as_cost`
+/// static (Yasharn, Angel of Jubilation), and the filter itself. Callers must not
+/// re-derive this — an AI-side copy that omits the static check over-counts
+/// eligible fodder under a "players can't sacrifice" effect. `pub` so `phase-ai`'s
+/// cast-time cost gates share it with the payment path in `cost_payability.rs` and
+/// `casting_costs.rs`.
+pub fn find_eligible_sacrifice_targets(
     state: &GameState,
     player: PlayerId,
     source_id: ObjectId,

crates/engine/src/game/game_object.rs

@@ -2477,6 +2477,102 @@ impl GameObject {
         self.phase_status.is_phased_in()
     }
 
+    /// CR 712.8a + CR 708.2a: the mana value this permanent will show **once it
+    /// leaves the battlefield** — the quantity a caller pricing a battlefield exit
+    /// must use, and which `self.mana_cost` and `self.base_mana_cost` each get
+    /// wrong in a different case.
+    ///
+    /// # Scope: BATTLEFIELD EXIT ONLY. Deliberately not zone-parameterized.
+    ///
+    /// `zones::apply_zone_exit_cleanup` (`zones.rs:137`) restores the stashed face
+    /// through **three independent gates**, not one disjunction, and only two of the
+    /// three are unconditional:
+    ///
+    /// | flag | gate in `apply_zone_exit_cleanup` | at `from = Battlefield` |
+    /// |---|---|---|
+    /// | `transformed` (`:261`, CR 712.8a + CR 400.7) | no zone gate at all | reverts |
+    /// | `modal_back_face` (`:273`, CR 712.8a + CR 400.7) | `to != Stack && to != Battlefield` | reverts for any non-stack, non-battlefield destination |
+    /// | `face_down` (`:286`, CR 708.9) | `from == Battlefield \|\| (from == Stack && to != Battlefield)` | reverts |
+    ///
+    /// **Each flag INDEPENDENTLY reverts for `Battlefield -> Graveyard`** (CR
+    /// 701.21a's transition), which is what the disjunction below relies on. The
+    /// claim is deliberately per-flag: the disjunction also admits flag
+    /// COMBINATIONS, and one is real and handled in-tree — see the next section.
+    /// None of the three is exact for a battlefield-to-STACK move, where the
+    /// `modal_back_face` gate does not fire. A destination parameter is not added:
+    /// only the battlefield-exit value is proven, and a parameter with one proven
+    /// value is speculative generality. Add it when a second transition earns its
+    /// own per-flag proof.
+    ///
+    /// # The one flag combination that occurs, and why it is still correct
+    ///
+    /// A **flipped permanent that is then turned face down** (Ixidron, Cyber
+    /// Conversion — CR 712.16 does not cover flip cards, so this is legal) sets
+    /// `flipped` AND `face_down` at once, and the two statuses **share the single
+    /// `back_face` slot**. `effects::turn_face_down` (`turn_face_down.rs:66-69`)
+    /// keeps the FLIP stash there rather than overwriting it with a base snapshot,
+    /// and `zones::apply_zone_exit_cleanup` runs the CR 708.9 face-down restore
+    /// BEFORE the CR 710.4 flip revert (`zones.rs:300-309`) precisely so one slot
+    /// serves both.
+    ///
+    /// So for that object `back_face` holds the flip card's NORMAL half, not the
+    /// base face — and reading it is not merely harmless, it is REQUIRED. Turning
+    /// the permanent face down set both `mana_cost` and `base_mana_cost` to
+    /// `ManaCost::NoCost` (`morph.rs:47`, CR 708.2a), so the `None =>` arm would
+    /// return 0 here. The `face_down` disjunct is what routes this object to the
+    /// stash instead. CR 710.1c ("A flip card's color and mana cost don't change if
+    /// the permanent is flipped") makes that stash mana-cost-identical to the
+    /// printed cost, so the number returned is right. Pinned by F8-E arm (e).
+    ///
+    /// `flipped` is **not** a fourth conjunct, and this is settled — do not
+    /// re-chase it. CR 710.1c again: `flip::apply_flipped_face_to_object`
+    /// (`flip.rs:320`) leaves `mana_cost` and `base_mana_cost` untouched by design
+    /// (`flip.rs:363-365`), so for a flipped-but-face-UP permanent the `None =>`
+    /// arm already returns the right number.
+    ///
+    /// # Why not `self.mana_cost`
+    ///
+    /// It is the live, layer-mutable characteristic (CR 613.1). `layers::
+    /// seed_live_characteristics_from_base` re-seeds it from `base_mana_cost` at the
+    /// top of every layer pass, and `printed_cards::apply_copiable_values`
+    /// (`printed_cards.rs:644`) then overwrites **only** the live field — it writes
+    /// no `base_*` field at all. CR 903.3's own example puts a copied commander in
+    /// scope here ("A commander that's copying another card … is still a commander").
+    ///
+    /// # Why not `self.base_mana_cost` alone
+    ///
+    /// `printed_cards::apply_back_face_to_object` (`:287`) writes **both** the live
+    /// (`:295`) and base (`:308`) fields from the installed face, and
+    /// `morph::apply_face_down_creature_characteristics` (`morph.rs:47`) sets both to
+    /// `ManaCost::NoCost` (CR 708.2a). For those objects `base_mana_cost` describes
+    /// the face currently shown, not the face the card will show off the battlefield.
+    ///
+    /// # Known residual, stated per flag rather than as a blanket claim
+    ///
+    /// The `back_face` slot is **shared** by the transform, MDFC, face-down and flip
+    /// stashes, and its writers do not agree on which snapshot they take:
+    ///   * `transform.rs:85`/`:90` stash `printed_cards::snapshot_object_face`, which
+    ///     captures the **live** `mana_cost` (`printed_cards.rs:717`). A permanent
+    ///     transformed while already under a mana-cost-altering copy effect therefore
+    ///     parks a polluted value, and this method will read it.
+    ///   * `effects/turn_face_down.rs:68` stashes `snapshot_object_base_face` — the
+    ///     **printed** baseline. That path is clean.
+    ///     The divergence is **BIDIRECTIONAL**, not downward-only: `intrinsic_copiable_
+    ///     values` (`printed_cards.rs:486`) sources `obj.base_mana_cost` from the COPY
+    ///     SOURCE and `apply_copiable_values` writes it to the RECIPIENT's live field
+    ///     with no clamp, so a `{1}{U}` Clone copying a fifteen-drop ends with live 15
+    ///     against base 2. Tracked as task #36; the fix is an engine change at the
+    ///     transform site and is out of this unit's scope.
+    pub fn mana_value_on_battlefield_exit(&self) -> u32 {
+        let shows_stashed_face = self.transformed || self.modal_back_face || self.face_down;
+        match self.back_face.as_ref().filter(|_| shows_stashed_face) {
+            Some(stashed) => stashed.mana_cost.mana_value(),
+            // CR 613.1: "For a card, [the characteristics are] the values printed on
+            // that card" — `base_mana_cost` is the engine's name for that baseline.
+            None => self.base_mana_cost.mana_value(),
+        }
+    }
+
     /// CR 702.26b: Whether this object is currently phased out (treated as
     /// though it doesn't exist for almost all rules queries).
     pub fn is_phased_out(&self) -> bool {
@@ -3685,4 +3781,93 @@ mod tests {
             "a Room card in hand combines both halves (CR 709.4b): {{U}} + {{4}}{{U}} = 6"
         );
     }
+
+    fn exit_value_fixture(id: u64, mana_value: u32) -> GameObject {
+        let mut obj = GameObject::new(
+            ObjectId(id),
+            CardId(id),
+            PlayerId(0),
+            "Exit Value Fixture".to_string(),
+            Zone::Battlefield,
+        );
+        obj.mana_cost = ManaCost::generic(mana_value);
+        obj.base_mana_cost = ManaCost::generic(mana_value);
+        obj
+    }
+
+    fn install_stashed_face(obj: &mut GameObject, mana_value: u32) {
+        let mut stashed = exit_value_fixture(obj.id.0 + 1000, mana_value);
+        stashed.name = "Stashed Face".to_string();
+        obj.back_face = Some(crate::game::printed_cards::snapshot_object_face(&stashed));
+    }
+
+    /// CR 712.8a + CR 708.2a: the exit accessor reads the restored face, not a
+    /// currently displayed transformed or face-down shell. The paired
+    /// flipped-only and flipped-plus-face-down arms pin CR 710.1c's distinction.
+    #[test]
+    fn mana_value_on_battlefield_exit_uses_the_face_that_will_be_restored() {
+        let mut transformed = exit_value_fixture(1, 0);
+        transformed.transformed = true;
+        install_stashed_face(&mut transformed, 5);
+        assert_eq!(
+            transformed.mana_value_on_battlefield_exit(),
+            5,
+            "transformed DFC restores its stashed face"
+        );
+
+        let mut face_down = exit_value_fixture(2, 0);
+        face_down.face_down = true;
+        face_down.mana_cost = ManaCost::NoCost;
+        face_down.base_mana_cost = ManaCost::NoCost;
+        install_stashed_face(&mut face_down, 4);
+        assert_eq!(
+            face_down.mana_value_on_battlefield_exit(),
+            4,
+            "face-down permanent restores its stashed identity"
+        );
+
+        let mut copied_cost_strip = exit_value_fixture(3, 4);
+        copied_cost_strip.mana_cost = ManaCost::NoCost;
+        assert_eq!(
+            copied_cost_strip.mana_value_on_battlefield_exit(),
+            4,
+            "a live cost change does not alter the printed exit cost"
+        );
+
+        let untouched = exit_value_fixture(4, 3);
+        assert_eq!(
+            untouched.mana_value_on_battlefield_exit(),
+            3,
+            "an untouched permanent uses its printed baseline"
+        );
+
+        let mut flipped_then_face_down = exit_value_fixture(5, 0);
+        flipped_then_face_down.flipped = true;
+        flipped_then_face_down.face_down = true;
+        flipped_then_face_down.mana_cost = ManaCost::NoCost;
+        flipped_then_face_down.base_mana_cost = ManaCost::NoCost;
+        install_stashed_face(&mut flipped_then_face_down, 2);
+        assert_eq!(
+            flipped_then_face_down.mana_value_on_battlefield_exit(),
+            2,
+            "the face-down gate must preserve a flipped card's normal-half stash"
+        );
+
+        let mut flipped_only = exit_value_fixture(6, 2);
+        flipped_only.flipped = true;
+        install_stashed_face(&mut flipped_only, 9);
+        assert_eq!(
+            flipped_only.mana_value_on_battlefield_exit(),
+            2,
+            "CR 710.1c: flipped-only does not route through the stash"
+        );
+
+        let mut missing_stash = exit_value_fixture(7, 4);
+        missing_stash.face_down = true;
+        assert_eq!(
+            missing_stash.mana_value_on_battlefield_exit(),
+            4,
+            "a missing stash falls back to the baseline rather than panicking"
+        );
+    }
 }

crates/engine/src/game/mana_abilities.rs

@@ -67,6 +67,57 @@ pub fn is_mana_ability(ability_def: &AbilityDefinition) -> bool {
     true
 }
 
+/// CR 701.21a: True when this ability's cost sacrifices **the source itself**.
+///
+/// Restricted to [`TargetFilter::SelfRef`] on purpose: a sac-outlet mana ability
+/// that eats *other* permanents (Skirk Prospector, "Sacrifice a Goblin: Add {R}")
+/// keeps its source on the battlefield and stays renewable. Delegates to
+/// [`mana_sources::cost_has_component`] so a **bare** `Sacrifice` cost and a
+/// `Sacrifice` nested in a `Composite` are both matched — the predefined Gold
+/// token prints the bare shape and Treasure the composite one.
+///
+/// Deliberately private, but privacy is a signpost rather than a barrier: it
+/// removes the *convenient* route to the naive per-permanent form ("has a mana
+/// ability AND no mana ability self-sacs" — which wrongly drops a permanent
+/// carrying both a renewable and a self-sacrificing mana ability), and does not
+/// make that form inexpressible. `AbilityCost::Sacrifice` and
+/// `TargetFilter::SelfRef` are both public, so any downstream crate can rebuild
+/// this predicate, and one did: `phase-ai` carried a hand-rolled
+/// `ability_cost_requires_sacrifice` whose own doc said it "mirrors
+/// `mana_sources::cost_requires_sacrifice` which is private to the engine
+/// module". It matched only the `Composite` shape, so Gold's **bare**
+/// `Sacrifice` hit its `_ => false` arm and a Gold token counted as renewable.
+/// That miscount — a re-implementation drifting from the rule — is the argument
+/// for keeping the classification here, not any guarantee privacy provides. The
+/// only exported composition is `.any(is_renewable_mana_ability)`, which *is*
+/// the per-ability filter.
+fn cost_sacrifices_self(cost: &Option<AbilityCost>) -> bool {
+    mana_sources::cost_has_component(
+        cost,
+        |c| matches!(c, AbilityCost::Sacrifice(s) if s.target == TargetFilter::SelfRef),
+    )
+}
+
+/// CR 605.1a + CR 701.21: a *renewable* mana ability — one that produces mana
+/// (per [`is_mana_ability`]) without consuming its own source to do it.
+///
+/// This is the **development** predicate: it answers "is this permanent part of a
+/// standing manabase," not "can this produce mana right now." A Treasure, Gold,
+/// Lotus Petal, Black Lotus, or Chromatic Star is a one-shot conversion of a
+/// permanent into mana and is deliberately **excluded**; Commander's Sphere,
+/// Powerstone, Springleaf Drum, and Skirk Prospector are **included**.
+///
+/// For live availability use [`is_mana_ability`] directly — an untapped Treasure
+/// genuinely is one mana available right now, which is why the two predicates must
+/// not be unified.
+///
+/// Takes a single ability so callers compose with `.any()`; a permanent counts if
+/// **at least one** of its mana abilities is renewable (Crystal Vein carries both
+/// a renewable `{T}: Add {C}` and a self-sac `{T}, Sac: Add {C}{C}`).
+pub fn is_renewable_mana_ability(ability_def: &AbilityDefinition) -> bool {
+    is_mana_ability(ability_def) && !cost_sacrifices_self(&ability_def.cost)
+}
+
 /// CR 605.1b: A triggered ability is a mana ability iff all three hold:
 ///   (a) it doesn't require a target (CR 115.6),
 ///   (b) it triggers from the activation/resolution of an activated mana ability
@@ -4296,6 +4347,81 @@ mod tests {
         }
     }
 
+    /// Skirk Prospector: "Sacrifice a Goblin: Add {R}". The sacrifice target is a
+    /// *type* filter, not `SelfRef`, so the source survives and stays renewable.
+    fn skirk_prospector_mana_ability() -> AbilityDefinition {
+        make_mana_ability(ManaProduction::Fixed {
+            colors: vec![ManaColor::Red],
+            contribution: ManaContribution::Base,
+        })
+        .cost(AbilityCost::Sacrifice(SacrificeCost::count(
+            TargetFilter::Typed(TypedFilter::new(TypeFilter::Subtype("Goblin".to_string()))),
+            1,
+        )))
+    }
+
+    /// Row 4a — CR 701.21: one-shot self-sacrificing mana sources are NOT
+    /// renewable. Gold is the constraint discriminator: its cost is a **bare**
+    /// `Sacrifice` (not wrapped in a `Composite` like Treasure's), so a
+    /// `Composite`-only implementation passes the Treasure assertion and fails
+    /// this one. Driven through the production `predefined_token_abilities`
+    /// materialization path, which is what actually lands on a live `GameObject`.
+    #[test]
+    fn treasure_and_gold_are_not_renewable_mana_abilities() {
+        let treasure = crate::game::effects::token::predefined_token_abilities("Treasure");
+        let gold = crate::game::effects::token::predefined_token_abilities("Gold");
+        assert_eq!(treasure.len(), 1);
+        assert_eq!(gold.len(), 1);
+
+        // Positive control: both ARE mana abilities. This proves the exclusion
+        // comes from the sacrifice clause and not from a failure to classify as
+        // a mana ability at all.
+        assert!(is_mana_ability(&treasure[0]));
+        assert!(is_mana_ability(&gold[0]));
+
+        assert!(
+            !is_renewable_mana_ability(&treasure[0]),
+            "Treasure ({{T}}, Sacrifice) is a one-shot source, not development"
+        );
+        assert!(
+            !is_renewable_mana_ability(&gold[0]),
+            "Gold's cost is a BARE Sacrifice — a Composite-only match misses it"
+        );
+    }
+
+    /// Row 4b — CR 701.21a: sacrificing *another* permanent leaves the source on
+    /// the battlefield, so a sac-outlet mana ability stays renewable. A
+    /// filter-agnostic implementation (reusing `cost_includes_sacrifice` /
+    /// `ManaSourcePenalty::Sacrifices`) fails this row.
+    #[test]
+    fn non_self_sacrifice_mana_outlet_stays_renewable() {
+        assert!(
+            is_renewable_mana_ability(&skirk_prospector_mana_ability()),
+            "\"Sacrifice a Goblin: Add {{R}}\" keeps its source — still development"
+        );
+
+        // Paired negative: the identical shape with a SelfRef filter is excluded.
+        let self_sac = make_mana_ability(ManaProduction::Fixed {
+            colors: vec![ManaColor::Red],
+            contribution: ManaContribution::Base,
+        })
+        .cost(AbilityCost::Sacrifice(SacrificeCost::count(
+            TargetFilter::SelfRef,
+            1,
+        )));
+        assert!(!is_renewable_mana_ability(&self_sac));
+    }
+
+    /// Row 4c — Powerstone and Treasure are both artifact tokens differing only in
+    /// cost shape (bare `Tap` vs `Composite{Tap, Sacrifice}`), so this proves the
+    /// discriminator is the sacrifice clause rather than the token-ness.
+    #[test]
+    fn powerstone_is_a_renewable_mana_ability() {
+        let powerstone = crate::game::effects::token::predefined_token_abilities("Powerstone");
+        assert_eq!(powerstone.len(), 1);
+        assert!(is_renewable_mana_ability(&powerstone[0]));
+    }
+
     #[test]
     fn mana_api_type_detected_as_mana_ability() {
         let def = make_mana_ability(ManaProduction::Fixed {

crates/engine/src/game/mulligan.rs

@@ -20,7 +20,7 @@ const STARTING_HAND_SIZE: usize = 7;
 /// hand would be zero cards. In a standard game that means at most 7 mulligans
 /// (7→6→5→4→3→2→1→0; the 8th would be 0). CR 103.5c adds that in free-first
 /// formats the first mulligan is uncounted, so the cap shifts up by one to 8
-/// — the player may still be brought all the way down to a 1-card opening
+/// — the player may still be brought all the way down to a 0-card opening
 /// hand after exhausting their bottoms allowance.
 const MAX_MULLIGANS: u8 = 7;
 

crates/engine/tests/integration/main.rs

@@ -758,6 +758,7 @@ mod ninjutsu_cluster;
 mod nissa_ascended_animist_loyalty_token_6012;
 mod nix_counter_no_mana_spent;
 mod no_witnesses_most_creatures_investigate;
+mod notion_thief_opponent_draw_redirect;
 mod nth_spell_ordinal_cost_reduction;
 mod ob_nixilis_captive_kingpin_life_loss;
 mod obeka_splitter_additional_phases;