fix(engine): preserve beginning combat priority

crates/engine/src/game/engine_auto_pass_decision_tests.rs

@@ -253,6 +253,57 @@ fn until_end_of_turn_finishes_at_configured_phase_stop() {
     assert!(is_finish(&priority_auto_pass_decision(&state, PlayerId(0))));
 }
 
+/// CR 507.2 + CR 117.3c: A beginning-of-combat phase stop interrupts an
+/// `UntilTurnBoundary` shortcut at a usable priority window. The non-mana
+/// activation proves this is a real priority window, not merely a rendered
+/// phase marker.
+#[test]
+fn begin_combat_phase_stop_interrupts_auto_pass_with_usable_priority() {
+    let mut state = priority_state();
+    let artifact = add_non_mana_activated_artifact(&mut state, PlayerId(0));
+    state.auto_pass.insert(
+        PlayerId(0),
+        AutoPassMode::UntilTurnBoundary {
+            until: TurnBoundary::EndOfCurrentTurn,
+        },
+    );
+    state.phase_stops.insert(
+        PlayerId(0),
+        vec![stop(Phase::BeginCombat, PhaseStopScope::OwnTurn)],
+    );
+
+    apply_as_current(&mut state, GameAction::PassPriority).unwrap();
+    let at_begin_combat = apply_as_current(&mut state, GameAction::PassPriority).unwrap();
+
+    assert_eq!(state.phase, Phase::BeginCombat);
+    assert!(matches!(
+        at_begin_combat.waiting_for,
+        WaitingFor::Priority {
+            player: PlayerId(0)
+        }
+    ));
+    assert!(
+        !state.auto_pass.contains_key(&PlayerId(0)),
+        "the explicit stop must interrupt the standing auto-pass session"
+    );
+
+    let activated = apply_as_current(
+        &mut state,
+        GameAction::ActivateAbility {
+            source_id: artifact,
+            ability_index: 0,
+        },
+    )
+    .expect("a non-mana activated ability is legal in the stopped BeginCombat window");
+    assert_eq!(state.stack.len(), 1);
+    assert!(matches!(
+        activated.waiting_for,
+        WaitingFor::Priority {
+            player: PlayerId(0)
+        }
+    ));
+}
+
 /// V8: the per-window interrupt logic is boundary-agnostic. A
 /// `MyNextTurnStart` session must Pass/Finish in exactly the same windows as
 /// the `EndOfCurrentTurn` sessions above (empty stack → Pass, opponent stack →

crates/engine/src/game/engine_phase_trigger_regression_tests.rs

@@ -17,6 +17,7 @@ use crate::types::format::FormatConfig;
 use crate::types::identifiers::{CardId, ObjectId};
 use crate::types::keywords::Keyword;
 use crate::types::mana::{ManaColor, ManaCost, ManaType, ManaUnit};
+use crate::types::phase::{PhaseStop, PhaseStopScope};
 use crate::types::player::PlayerId;
 use crate::types::replacements::ReplacementEvent;
 use crate::types::triggers::TriggerMode;
@@ -84,11 +85,11 @@ fn hand_to_battlefield_choice_ability(
     )
 }
 
-/// Verify that combat is skipped when there are no attackers and no triggers.
-/// With no BeginCombat triggers and no potential attackers, auto_advance()
-/// skips straight to PostCombatMain.
+/// CR 507.2 + CR 508.1a: Even with no attackers and no beginning-of-combat
+/// triggers, beginning of combat has an active-player priority window before
+/// the active player makes the (here forced-empty) attacker declaration.
 #[test]
-fn combat_skipped_when_no_attackers_no_triggers() {
+fn begin_combat_window_precedes_forced_empty_attacker_declaration() {
     let mut state = new_game(42);
     state.turn_number = 2;
     state.phase = Phase::PreCombatMain;
@@ -98,24 +99,23 @@ fn combat_skipped_when_no_attackers_no_triggers() {
         player: PlayerId(0),
     };
 
-    // Create a 0/1 creature with no triggers — can't attack, no combat triggers.
-    let creature_id = create_object(
-        &mut state,
-        CardId(200),
+    // Stops make both windows observable; otherwise the forced empty
+    // declaration is deliberately auto-submitted by the normal auto-pass loop.
+    state.phase_stops.insert(
         PlayerId(0),
-        "Wall".to_string(),
-        Zone::Battlefield,
+        vec![
+            PhaseStop {
+                phase: Phase::BeginCombat,
+                scope: PhaseStopScope::OwnTurn,
+            },
+            PhaseStop {
+                phase: Phase::DeclareAttackers,
+                scope: PhaseStopScope::OwnTurn,
+            },
+        ],
     );
-    {
-        let obj = state.objects.get_mut(&creature_id).unwrap();
-        obj.card_types.core_types.push(CoreType::Creature);
-        obj.power = Some(0);
-        obj.toughness = Some(1);
-    }
 
-    // Pass priority twice (P0 passes, then P1 passes) with empty stack.
-    // This advances from PreCombatMain → BeginCombat → no triggers, no
-    // attackers → skip to PostCombatMain.
+    // Passing the precombat-main priority window reaches beginning of combat.
     let result1 = apply_as_current(&mut state, GameAction::PassPriority).unwrap();
     assert!(matches!(
         result1.waiting_for,
@@ -126,18 +126,85 @@ fn combat_skipped_when_no_attackers_no_triggers() {
 
     let result2 = apply_as_current(&mut state, GameAction::PassPriority).unwrap();
 
-    // We should now be at PostCombatMain with empty stack.
-    assert_eq!(state.phase, Phase::PostCombatMain);
+    assert_eq!(state.phase, Phase::BeginCombat);
+    assert!(matches!(
+        result2.waiting_for,
+        WaitingFor::Priority {
+            player: PlayerId(0)
+        }
+    ));
     assert!(
         state.stack.is_empty(),
-        "Stack should be empty — no triggers exist. Stack: {:?}",
+        "the no-trigger window must not fabricate stack work: {:?}",
         state.stack
     );
+    assert!(state.pending_trigger.is_none());
+
+    // Both players then pass the mandated beginning-of-combat window. The
+    // downstream DeclareAttackers prompt remains visible despite its forced
+    // empty declaration because the explicit stop overrides auto-submit.
+    apply_as_current(&mut state, GameAction::PassPriority).unwrap();
+    let result4 = apply_as_current(&mut state, GameAction::PassPriority).unwrap();
+    assert_eq!(state.phase, Phase::DeclareAttackers);
+    assert!(matches!(
+        result4.waiting_for,
+        WaitingFor::DeclareAttackers {
+            player: PlayerId(0),
+            ref valid_attacker_ids,
+            ..
+        } if valid_attacker_ids.is_empty()
+    ));
+
+    // Submit the only legal declaration through the production action path.
+    // CR 508.8 then skips only DeclareBlockers and CombatDamage, leaving combat
+    // and arriving at PostCombatMain normally.
+    let empty_declaration = apply_as_current(
+        &mut state,
+        GameAction::DeclareAttackers {
+            attacks: vec![],
+            bands: vec![],
+        },
+    )
+    .expect("the empty declaration offered by the engine must be submitable");
+    assert_eq!(state.phase, Phase::PostCombatMain);
     assert!(
-        state.pending_trigger.is_none(),
-        "No pending trigger should exist"
+        state.combat.is_none(),
+        "combat ends after the empty declaration"
     );
-    assert!(matches!(result2.waiting_for, WaitingFor::Priority { .. }));
+    assert!(matches!(
+        empty_declaration.waiting_for,
+        WaitingFor::Priority {
+            player: PlayerId(0)
+        }
+    ));
+}
+
+/// CR 500.8 + CR 507.2: An inserted combat phase gets the same
+/// beginning-of-combat priority window as the natural combat phase.
+#[test]
+fn inserted_begin_combat_gets_priority_window() {
+    let mut state = setup_game_at_main_phase();
+    state.phase = Phase::EndCombat;
+    state
+        .extra_phases
+        .push(crate::types::game_state::ExtraPhase {
+            anchor: Phase::EndCombat,
+            phase: Phase::BeginCombat,
+            attacker_restriction: None,
+            attacker_restriction_source: None,
+        });
+
+    let mut events = Vec::new();
+    let waiting_for = crate::game::turns::auto_advance(&mut state, &mut events);
+
+    assert_eq!(state.phase, Phase::BeginCombat);
+    assert!(state.combat.is_some());
+    assert!(matches!(
+        waiting_for,
+        WaitingFor::Priority {
+            player: PlayerId(0)
+        }
+    ));
 }
 
 /// CR 503.1a: Upkeep triggers fire when the upkeep step begins.
@@ -246,6 +313,61 @@ fn begin_combat_trigger_fires_with_attackers() {
     );
 }
 
+/// CR 603.3b + CR 507.2: A phase-trigger ordering prompt is a stronger result
+/// than the ordinary beginning-of-combat priority window and must propagate
+/// unchanged through the phase interpreter.
+#[test]
+fn begin_combat_propagates_generic_phase_trigger_ordering_prompt() {
+    let mut state = setup_game_at_main_phase();
+    for (card_id, amount) in [(201_u64, 1), (202, 2)] {
+        let source_id = create_object(
+            &mut state,
+            CardId(card_id),
+            PlayerId(0),
+            format!("Combat trigger {card_id}"),
+            Zone::Battlefield,
+        );
+        state
+            .objects
+            .get_mut(&source_id)
+            .unwrap()
+            .card_types
+            .core_types
+            .push(CoreType::Creature);
+        let source = state.objects.get_mut(&source_id).unwrap();
+        source.power = Some(1);
+        source.toughness = Some(1);
+        source.trigger_definitions.push(
+            TriggerDefinition::new(TriggerMode::Phase)
+                .phase(Phase::BeginCombat)
+                .execute(AbilityDefinition::new(
+                    AbilityKind::Activated,
+                    Effect::GainLife {
+                        amount: QuantityExpr::Fixed { value: amount },
+                        player: TargetFilter::Controller,
+                    },
+                ))
+                .trigger_zones(vec![Zone::Battlefield]),
+        );
+    }
+
+    apply_as_current(&mut state, GameAction::PassPriority).unwrap();
+    let result = apply_as_current(&mut state, GameAction::PassPriority).unwrap();
+
+    assert_eq!(state.phase, Phase::BeginCombat);
+    assert!(
+        state.combat.is_some(),
+        "combat state is available to the prompt"
+    );
+    assert!(matches!(
+        result.waiting_for,
+        WaitingFor::OrderTriggers {
+            player: PlayerId(0),
+            ref triggers,
+        } if triggers.len() == 2
+    ));
+}
+
 /// CR 507.1: BeginCombat triggers fire even without potential attackers.
 #[test]
 fn begin_combat_trigger_fires_without_attackers() {

crates/engine/src/game/engine_tests.rs

@@ -3966,9 +3966,14 @@ fn integration_full_turn_cycle() {
         }
     ));
 
-    // Pass priority from player 1 (both passed, stack empty -> advance)
+    // Pass priority from player 1 (both passed, stack empty -> BeginCombat).
     let _result = apply_as_current(&mut state, GameAction::PassPriority).unwrap();
-    // Should skip combat phases and land at PostCombatMain
+    assert_eq!(state.phase, Phase::BeginCombat);
+
+    // Beginning of combat has its own priority window. With no attackers, the
+    // subsequent forced empty declaration skips only blockers and damage.
+    apply_as_current(&mut state, GameAction::PassPriority).unwrap();
+    apply_as_current(&mut state, GameAction::PassPriority).unwrap();
     assert_eq!(state.phase, Phase::PostCombatMain);
 
     // Pass through post-combat main
@@ -5915,7 +5920,12 @@ fn full_turn_integration_with_mulligan() {
     // Pass priority through the rest of the turn
     // PreCombatMain: P0 passes
     apply_as_current(&mut state, GameAction::PassPriority).unwrap();
-    // PreCombatMain: P1 passes -> advances to PostCombatMain
+    // PreCombatMain: P1 passes -> BeginCombat priority.
+    apply_as_current(&mut state, GameAction::PassPriority).unwrap();
+    assert_eq!(state.phase, Phase::BeginCombat);
+    // BeginCombat: both pass. No attackers are declared, so only Declare
+    // Blockers and Combat Damage are skipped before PostCombatMain.
+    apply_as_current(&mut state, GameAction::PassPriority).unwrap();
     apply_as_current(&mut state, GameAction::PassPriority).unwrap();
     assert_eq!(state.phase, Phase::PostCombatMain);
 

crates/engine/src/game/scenario.rs

@@ -1310,6 +1310,29 @@ impl GameRunner {
 
     /// Execute a single action. Returns the `ActionResult` from the engine.
     pub fn act(&mut self, action: GameAction) -> Result<ActionResult, EngineError> {
+        // Test scenarios historically modelled the transition out of precombat
+        // main as directly reaching DeclareAttackers. CR 507.2 now exposes the
+        // intervening priority window, so preserve that test-driver shorthand
+        // by passing the window only when a scenario submits its declaration.
+        // Live callers use `engine::apply` and must act during that window.
+        if matches!(&action, GameAction::DeclareAttackers { .. })
+            && self.state.phase == Phase::BeginCombat
+            && matches!(self.state.waiting_for, WaitingFor::Priority { .. })
+            && self.state.stack.is_empty()
+        {
+            let mut pass_events = Vec::new();
+            while self.state.phase == Phase::BeginCombat
+                && matches!(self.state.waiting_for, WaitingFor::Priority { .. })
+                && self.state.stack.is_empty()
+            {
+                pass_events
+                    .extend(apply_as_current(&mut self.state, GameAction::PassPriority)?.events);
+            }
+            let mut result = apply_as_current(&mut self.state, action)?;
+            pass_events.append(&mut result.events);
+            result.events = pass_events;
+            return Ok(result);
+        }
         apply_as_current(&mut self.state, action)
     }
 
@@ -1456,22 +1479,19 @@ impl GameRunner {
         self.advance_to_phase(Phase::Upkeep);
     }
 
-    /// Declare attackers (CR 508.1). Must be called when the engine is at
-    /// `WaitingFor::DeclareAttackers` (use [`GameRunner::advance_to_combat`]).
+    /// Declare attackers (CR 508.1). Accepts the scenario driver's established
+    /// shorthand for passing an empty beginning-of-combat priority window.
     /// Each entry is `(attacker, defender)` where `defender` is an
     /// [`AttackTarget`](crate::game::combat::AttackTarget) — a player,
     /// planeswalker, or battle (CR 508.1b).
     pub fn declare_attackers(
         &mut self,
         attacks: &[(ObjectId, crate::game::combat::AttackTarget)],
     ) -> Result<ActionResult, EngineError> {
-        apply_as_current(
-            &mut self.state,
-            GameAction::DeclareAttackers {
-                attacks: attacks.to_vec(),
-                bands: vec![],
-            },
-        )
+        self.act(GameAction::DeclareAttackers {
+            attacks: attacks.to_vec(),
+            bands: vec![],
+        })
     }
 
     /// CR 702.103b: put `attachment` onto `host` in its BESTOWED AURA FORM —