fix(engine): release trigger carrier on drop and unstick empty attackers

crates/engine/src/game/combat.rs

@@ -7367,6 +7367,15 @@ mod tests {
 
         // Non-sick, eligible creature (create_creature leaves summoning_sick false).
         let id = create_creature(&mut state, PlayerId(0), "Bear", 2, 2);
+        // CR 508.1a: a SECOND eligible attacker keeps the refreshed snapshot
+        // non-empty after `id` goes sick. Without it the declaration becomes
+        // forced, and `run_auto_pass_loop` auto-submits the only legal (empty)
+        // declaration — the prompt is gone and this row can no longer observe
+        // the refresh at all. Keeping the set non-empty also makes the
+        // assertion strictly sharper: it proves the refresh dropped THAT
+        // creature while retaining the other, which an emptied set cannot
+        // distinguish from a snapshot that simply cleared everything.
+        let companion = create_creature(&mut state, PlayerId(0), "Ox", 3, 3);
 
         state.waiting_for = WaitingFor::DeclareAttackers {
             player: PlayerId(0),
@@ -7378,10 +7387,17 @@ mod tests {
         match &state.waiting_for {
             WaitingFor::DeclareAttackers {
                 valid_attacker_ids, ..
-            } => assert!(
-                valid_attacker_ids.contains(&id),
-                "precondition: eligible creature must be a valid attacker"
-            ),
+            } => {
+                assert!(
+                    valid_attacker_ids.contains(&id),
+                    "precondition: eligible creature must be a valid attacker"
+                );
+                assert!(
+                    valid_attacker_ids.contains(&companion),
+                    "precondition: the companion must also be a valid attacker, \
+                     or the post-refresh set would be empty for the wrong reason"
+                );
+            }
             other => panic!("expected DeclareAttackers, got {other:?}"),
         }
 
@@ -7398,10 +7414,17 @@ mod tests {
         match &result.waiting_for {
             WaitingFor::DeclareAttackers {
                 valid_attacker_ids, ..
-            } => assert!(
-                !valid_attacker_ids.contains(&id),
-                "refreshed snapshot must drop the now-sick creature"
-            ),
+            } => {
+                assert!(
+                    !valid_attacker_ids.contains(&id),
+                    "refreshed snapshot must drop the now-sick creature"
+                );
+                assert!(
+                    valid_attacker_ids.contains(&companion),
+                    "the refresh must be selective, not a wholesale clear: the \
+                     untouched companion is still an eligible attacker"
+                );
+            }
             other => panic!("expected refreshed DeclareAttackers, got {other:?}"),
         }
     }

crates/engine/src/game/engine.rs

@@ -6454,10 +6454,25 @@ fn run_auto_pass_loop(state: &mut GameState, result: &mut ActionResult) -> bool
                 }
             }
 
-            // UntilTurnBoundary: auto-submit empty attackers unless the user
-            // flagged this phase as a stop.
-            WaitingFor::DeclareAttackers { player, .. }
-                if end_of_turn_active(state, *player) && !state.phase_stop_hit(*player) =>
+            // CR 508.1a: "The active player chooses which creatures that they
+            // control, IF ANY, will attack." When no creature can legally attack,
+            // the empty declaration is the ONLY legal declaration — there is no
+            // choice to make. Parking on the prompt strands the game on a decision
+            // whose entire legal-action set is a single no-op submission, which the
+            // player must click through every combat (and which an automated seat
+            // may never submit at all). Mirrors the `DeclareBlockers` arm below,
+            // which already auto-submits whenever there is nothing to choose.
+            //
+            // `UntilTurnBoundary` additionally auto-submits when candidates DO
+            // exist — that is the player's standing pre-commitment to attack with
+            // nothing. A phase stop overrides both: an explicit request to pause
+            // here is honored even when the declaration is forced.
+            WaitingFor::DeclareAttackers {
+                player,
+                valid_attacker_ids,
+                ..
+            } if !state.phase_stop_hit(*player)
+                && (valid_attacker_ids.is_empty() || end_of_turn_active(state, *player)) =>
             {
                 let mut events = Vec::new();
                 match engine_combat::handle_empty_attackers(state, &mut events) {
@@ -11508,15 +11523,30 @@ fn apply_retarget(
     Ok(state.waiting_for.clone())
 }
 
-/// CR 603.3c + CR 608.2c: Drop a mid-construction optional triggered modal that
-/// was declined before mode choice.
+/// CR 603.3c + CR 603.3d + CR 608.2c: Single authority for dropping a
+/// mid-construction triggered ability — an optional modal declined before mode
+/// choice, or CR 603.3d's "if a choice is required when the triggered ability
+/// goes on the stack but no legal choices can be made for it ... the ability is
+/// simply removed from the stack."
+///
+/// Every in-flight construction cursor must be released together. The event
+/// batch is the paused trigger's carrier: `begin_pending_trigger_target_selection`
+/// re-reads `pending_trigger_event_batch` as the event context for whichever
+/// trigger is being constructed, and the pause paths write it straight back. A
+/// drop that clears the trigger but leaks the batch therefore leaves a dead
+/// event latched in state, where it (a) poisons the event context of every
+/// later trigger that pauses for a choice, and (b) permanently fails the
+/// `inert_trigger_batch_state_is_settled` gate that lets contiguous inert
+/// trigger runs skip priority. Mirrors `triggers::abandon_ceased_pending_trigger`,
+/// which already releases all four cursors on the error-recovery path.
 pub(super) fn drop_mid_construction_pending_trigger(state: &mut GameState) {
     super::stack::pop_uncommitted_pending_trigger_entry(
         state,
         super::lifecycle::DelayedTerminalDisposition::NoLegalChoice,
     );
     state.pending_trigger = None;
     state.pending_trigger_firing = None;
+    state.pending_trigger_event_batch.clear();
 }
 
 /// Clear optionality after the controller accepts a "you may choose N" gate so
@@ -11610,12 +11640,7 @@ pub(super) fn begin_pending_trigger_target_selection(
                 &mode_abilities,
                 &unavailable_modes,
             ) else {
-                super::stack::pop_uncommitted_pending_trigger_entry(
-                    state,
-                    super::lifecycle::DelayedTerminalDisposition::NoLegalChoice,
-                );
-                state.pending_trigger = None;
-                state.pending_trigger_firing = None;
+                drop_mid_construction_pending_trigger(state);
                 return Ok(None);
             };
 
@@ -11633,12 +11658,7 @@ pub(super) fn begin_pending_trigger_target_selection(
                  dispatch_pending_trigger_context must resolve it inline",
             );
             if modal.selection.is_random() {
-                super::stack::pop_uncommitted_pending_trigger_entry(
-                    state,
-                    super::lifecycle::DelayedTerminalDisposition::NoLegalChoice,
-                );
-                state.pending_trigger = None;
-                state.pending_trigger_firing = None;
+                drop_mid_construction_pending_trigger(state);
                 return Ok(None);
             }
 
@@ -11651,12 +11671,7 @@ pub(super) fn begin_pending_trigger_target_selection(
             // dead branch — kept as a defensive cleanup for any
             // delayed-revalidation paths.
             if unavailable_modes.len() >= modal.mode_count {
-                super::stack::pop_uncommitted_pending_trigger_entry(
-                    state,
-                    super::lifecycle::DelayedTerminalDisposition::NoLegalChoice,
-                );
-                state.pending_trigger = None;
-                state.pending_trigger_firing = None;
+                drop_mid_construction_pending_trigger(state);
                 return Ok(None);
             }
 
@@ -11770,12 +11785,7 @@ pub(super) fn begin_pending_trigger_target_selection(
         // branch above: if the "push first" dispatcher already pushed an
         // in-construction entry for this trigger, pop it before clearing the
         // cursor.
-        super::stack::pop_uncommitted_pending_trigger_entry(
-            state,
-            super::lifecycle::DelayedTerminalDisposition::NoLegalChoice,
-        );
-        state.pending_trigger = None;
-        state.pending_trigger_firing = None;
+        drop_mid_construction_pending_trigger(state);
         return Ok(None);
     };
     Ok(Some(WaitingFor::TriggerTargetSelection {
@@ -15654,7 +15664,31 @@ mod stage2_injector_tests {
                 // PR #7041's typed trigger-provenance initializers sit above the
                 // first three effects producers and this engine producer. CI
                 // re-derived the same five writes at these coordinates.
-                "game/engine.rs:11697".to_string(),
+                //
+                // CR 603.3d CARRIER-RELEASE FIX: `:11697 ⇒ :11712`. Pure line movement from
+                // two edits in this file, neither of which mints a prompt: the empty-attackers
+                // auto-submit guard in `run_auto_pass_loop` (`:6457`, +15) and the collapse of
+                // four duplicated mid-construction drop blocks into calls to
+                // `drop_mid_construction_pending_trigger` (`:11511` +14 for its doc comment,
+                // `:11519` +1 for the `pending_trigger_event_batch.clear()` line, then -5 at
+                // each of `:11613`/`:11636`/`:11654`). Those six hunks sum to +15 and
+                // 11697 + 15 = 11712 exactly. The fourth collapsed block (`:11773`, also -5)
+                // sits BELOW this producer and therefore cannot move it, which is why the sum
+                // is +15 rather than the whole-file +10.
+                //
+                // LOCATED BY CONTENT, as this log requires: `:11712` hashes to sha256
+                // `8a544e878d3e77fb…`, the same prefix carried for this producer since
+                // `a6d1a0e62`, and it is the ONLY line in the file matching the producer shape
+                // outside `#[cfg(test)]`. Still inside `begin_pending_trigger_target_selection`,
+                // which moved `:11548 ⇒ :11578` by the +30 of the three hunks above the function
+                // itself — the same arithmetic re-derived against a different anchor.
+                //
+                // SET PRESERVATION: the other four entries are byte-identical AND in place
+                // (`effects/` and `scoped_library_search.rs` are untouched by this change), and
+                // the two tests this change adds contain no line matching the needle, so the
+                // total stays 37 and the partition stays 5/7/25. A drop path releasing a
+                // construction cursor cannot mint a CR 603.5 prompt.
+                "game/engine.rs:11712".to_string(),
             ],
             "the five production producers, NAMED: the CR 603.5 gate in `resolve_chain_body` \
              plus the two repeated-optional-payment drivers, the per-player acceptance cursor \

crates/engine/src/game/engine_auto_pass_decision_tests.rs

@@ -1029,3 +1029,56 @@ fn loop_gate_probes_all_living_players_not_just_current_holder() {
              wrongly clear, proving the all-players probe is load-bearing"
     );
 }
+
+/// CR 508.1a: "The active player chooses which creatures that they control, IF
+/// ANY, will attack." When the candidate set is empty there is no choice to
+/// make — the empty declaration is the only legal one — so the engine must
+/// submit it rather than park on the prompt.
+///
+/// Regression: this arm previously auto-submitted ONLY when the player was in
+/// `AutoPassMode::UntilTurnBoundary`. A player with no auto-pass configured and
+/// no creatures therefore sat on a Declare Attackers prompt whose entire legal
+/// action set was a single no-op `DeclareAttackers { attacks: [], bands: [] }`,
+/// which had to be clicked through every combat. The `DeclareBlockers` arm has
+/// always carried the equivalent "nothing to choose" escape; this pins the
+/// attacker side to the same rule.
+#[test]
+fn empty_attacker_set_auto_submits_without_any_auto_pass_mode() {
+    let waiting_for = WaitingFor::DeclareAttackers {
+        player: PlayerId(0),
+        valid_attacker_ids: Vec::new(),
+        valid_attack_targets: Vec::new(),
+        valid_attack_targets_by_attacker: Some(Default::default()),
+        attacker_constraints: Default::default(),
+    };
+    let mut state = GameState::new_two_player(42);
+    state.phase = Phase::DeclareAttackers;
+    state.active_player = PlayerId(0);
+    state.priority_player = PlayerId(0);
+    // Production sets combat before advancing into the declare step.
+    state.combat = Some(crate::game::combat::CombatState::default());
+    state.waiting_for = waiting_for.clone();
+
+    // The stalling configuration: no auto-pass mode for the declaring player.
+    assert!(
+        state.auto_pass.is_empty(),
+        "fixture must exercise the no-auto-pass case that stalled"
+    );
+
+    let mut result = ActionResult {
+        events: Vec::new(),
+        waiting_for,
+        log_entries: Vec::new(),
+    };
+    let advanced = run_auto_pass_loop(&mut state, &mut result);
+
+    assert!(
+        advanced,
+        "CR 508.1a: a forced empty attack declaration must not park the game"
+    );
+    assert!(
+        !matches!(result.waiting_for, WaitingFor::DeclareAttackers { .. }),
+        "CR 508.1a: the forced empty declaration must be submitted, not re-offered; got {:?}",
+        result.waiting_for
+    );
+}

crates/engine/src/game/engine_trigger_target_tests.rs

@@ -1265,3 +1265,107 @@ fn modal_mode_tracking_resets_on_new_turn() {
     // Game-scoped should persist.
     assert!(state.modal_modes_chosen_this_game.contains(&(source_id, 0)));
 }
+
+/// CR 603.3d: "If a choice is required when the triggered ability goes on the
+/// stack but no legal choices can be made for it ... the ability is simply
+/// removed from the stack." Removing it must release EVERY in-flight
+/// construction cursor, including `pending_trigger_event_batch`.
+///
+/// Regression: Nimble Obstructionist ("When you cycle this card, counter target
+/// activated or triggered ability you don't control") cycled with nothing legal
+/// to counter took this drop path. The drop cleared `pending_trigger` and
+/// `pending_trigger_firing` but leaked the batch, latching a dead `Cycled`
+/// event into the game state permanently — it then poisoned the trigger event
+/// context of every later trigger that paused for a choice (firing "whenever a
+/// player cycles" / "whenever you draw" observers for a cycle that never
+/// happened) and permanently failed the settled-state gate that lets contiguous
+/// inert trigger runs skip priority.
+#[test]
+fn no_legal_target_trigger_drop_releases_pending_trigger_event_batch() {
+    let mut state = GameState::new_two_player(42);
+    state.turn_number = 2;
+    state.phase = Phase::PreCombatMain;
+    state.active_player = PlayerId(0);
+    state.priority_player = PlayerId(0);
+
+    let source_id = create_object(
+        &mut state,
+        CardId(20),
+        PlayerId(0),
+        "Cycled Trigger Source".to_string(),
+        Zone::Graveyard,
+    );
+
+    // The battlefield is deliberately empty, so a creature-targeting trigger has
+    // no legal target at choose-time — the CR 603.3d removal branch.
+    let ability = ResolvedAbility::new(
+        Effect::DealDamage {
+            amount: QuantityExpr::Fixed { value: 1 },
+            target: TargetFilter::Typed(TypedFilter::creature()),
+            damage_source: None,
+            excess: None,
+        },
+        Vec::new(),
+        source_id,
+        PlayerId(0),
+    );
+
+    let cycled_event = GameEvent::Cycled {
+        player_id: PlayerId(0),
+        object_id: source_id,
+    };
+    let pending = crate::game::triggers::PendingTrigger {
+        source_id,
+        controller: PlayerId(0),
+        condition: None,
+        ability: Box::new(ability),
+        timestamp: 1,
+        target_constraints: Vec::new(),
+        distribute: None,
+        trigger_event: Some(cycled_event.clone()),
+        modal: None,
+        mode_abilities: vec![],
+        description: Some("When you cycle this card, counter target ability".to_string()),
+        may_trigger_origin: None,
+        subject_match_count: None,
+        die_result: None,
+        provenance: None,
+    };
+    let pending_for_state = pending.clone();
+    let mut setup_events = Vec::new();
+    let entry_id = crate::game::triggers::push_pending_trigger_to_stack(
+        &mut state,
+        pending,
+        &mut setup_events,
+    );
+    state.pending_trigger = Some(Box::new(pending_for_state));
+    mark_ordinary_pending_trigger_construction(&mut state, entry_id);
+    // Production installs the carrier AFTER the push (the push drains it), which
+    // is exactly the state a paused construction is re-entered in.
+    state.pending_trigger_event_batch = vec![cycled_event];
+
+    // Non-vacuity guard: the assertion below is only meaningful if the carrier is
+    // actually populated going in. `push_pending_trigger_to_stack` DRAINS the
+    // batch, so a future reordering of this fixture would silently turn the
+    // regression assert into a tautology that passes with the fix reverted.
+    assert!(
+        !state.pending_trigger_event_batch.is_empty(),
+        "fixture must enter the drop path with a populated carrier"
+    );
+
+    let waiting = crate::game::engine::begin_pending_trigger_target_selection(&mut state)
+        .expect("no-legal-target drop is not an engine error");
+
+    assert!(
+        waiting.is_none(),
+        "CR 603.3d: a trigger with no legal target must not surface a prompt"
+    );
+    assert!(
+        state.pending_trigger_event_batch.is_empty(),
+        "CR 603.3d: removing the ability must release its event-batch carrier, \
+         not latch a dead event into the game state"
+    );
+    assert!(state.pending_trigger.is_none());
+    assert!(state.pending_trigger_entry.is_none());
+    assert!(state.pending_trigger_firing.is_none());
+}