diff --git a/crates/engine/src/game/effects/end_phase.rs b/crates/engine/src/game/effects/end_phase.rs index 085504f101..7507c6e5be 100644 --- a/crates/engine/src/game/effects/end_phase.rs +++ b/crates/engine/src/game/effects/end_phase.rs @@ -1,4 +1,5 @@ use crate::game::effects::change_zone::{self, ZoneMoveResult}; +use crate::game::stack::pop_top_stack_entry; use crate::types::events::GameEvent; use crate::types::game_state::{GameState, StackEntryKind}; use crate::types::identifiers::ObjectId; @@ -25,9 +26,8 @@ pub(super) fn exile_nonresolving_stack_objects( source_id: ObjectId, events: &mut Vec, ) -> bool { - while let Some(entry) = state.stack.pop_back() { - state.stack_paid_facts.remove(&entry.id); - state.stack_trigger_event_batches.remove(&entry.id); + while let Some(removed) = pop_top_stack_entry(state) { + let entry = removed.entry; if matches!(entry.kind, StackEntryKind::Spell { .. }) { match change_zone::execute_zone_move( state, diff --git a/crates/engine/src/game/stack.rs b/crates/engine/src/game/stack.rs index c30e37a9b2..8b40f7224a 100644 --- a/crates/engine/src/game/stack.rs +++ b/crates/engine/src/game/stack.rs @@ -16,7 +16,8 @@ use crate::types::identifiers::ObjectId; use crate::types::player::PlayerId; use crate::types::resolved_commands::{ ResolvedStackEntryFinalizeCommand, ResolvedStackEntryFinalizeReplayInvariantError, - ResolvedStackPushCommand, ResolvedStackPushOrigin, ResolvedStackPushReplayInvariantError, + ResolvedStackPopCommand, ResolvedStackPopReplayInvariantError, ResolvedStackPushCommand, + ResolvedStackPushOrigin, ResolvedStackPushReplayInvariantError, ResolvedUncommittedTriggerRemovalCommand, ResolvedUncommittedTriggerRemovalReplayInvariantError, }; @@ -269,6 +270,96 @@ pub fn apply_resolved_stack_entry_finalize( Ok(()) } +/// Everything one stack removal settles: the entry plus the per-entry side-table +/// rows keyed on it. +/// +/// The rows are returned rather than discarded because the resolution pop +/// consumes both — the paid snapshot feeds cost-dependent resolution and the +/// batch feeds CR 603.7c event context. Callers that only need the entry drop +/// the rest. +pub(crate) struct PoppedStackEntry { + pub entry: StackEntry, + pub paid_facts: Option, + pub trigger_event_batch: Option>, +} + +/// CR 405.2: removes the topmost object from the stack. +/// +/// The single authority for the ordinary top-of-stack removal — the CR 405.5 +/// resolution pop and the drain loops that clear several entries in one pass +/// (batched resolution, inert no-op batches, CR 724.1b stack exile). Each call +/// removes exactly one object, so a drain of N entries is N removals rather +/// than one bulk mutation. +/// +/// Both side tables are dropped here rather than by the callers because they are +/// keyed on the entry and settle WITH the pop: a removal that dropped the entry +/// but left `stack_paid_facts` or `stack_trigger_event_batches` behind would +/// strand rows against an id no longer on the stack. +/// +/// NOT used by [`pop_uncommitted_pending_trigger_entry`], which performs the +/// same three-line mutation. That is deliberate: the CR 603.3d removal is a +/// distinct family with its own record, and routing it through this authority +/// would journal one mutation twice, so a replay would pop two entries where +/// execution popped one. +pub(crate) fn pop_top_stack_entry(state: &mut GameState) -> Option { + let entry = state.stack.pop_back()?; + let paid_facts = state.stack_paid_facts.remove(&entry.id); + let trigger_event_batch = state.stack_trigger_event_batches.remove(&entry.id); + + // CR 733: journal once ALL THREE removals have settled, so the record + // describes a stack the entry has already left. An empty stack is the one + // case that journals nothing — `?` returns above, because no mutation + // happened at all. + let cause = state.current_or_begin_rules_execution_node(); + state + .resolved_rules_journal + .record_stack_pop(ResolvedStackPopCommand { + entry: Box::new(entry.clone()), + resulting_depth: state.stack.len(), + cause, + }) + .expect("resolved stack pop must have a live journal cause"); + + Some(PoppedStackEntry { + entry, + paid_facts, + trigger_event_batch, + }) +} + +/// Replays one already-resolved CR 405.2 stack pop. +/// +/// Installs the recorded removal with nothing re-derived: the entry to remove is +/// verified against the record rather than located by a fresh scan. Both +/// preconditions are checked BEFORE any mutation, so a rejected replay leaves +/// the stack and both side tables untouched. +pub fn apply_resolved_stack_pop( + state: &mut GameState, + command: &ResolvedStackPopCommand, +) -> Result<(), ResolvedStackPopReplayInvariantError> { + // CR 405.2: the predecessor must be exactly one deeper than the record. + let expected_depth = command.resulting_depth + 1; + if state.stack.len() != expected_depth { + return Err(ResolvedStackPopReplayInvariantError::DepthMismatch { + expected: expected_depth, + found: state.stack.len(), + }); + } + // Compared WHOLE rather than by id: an applier that matched on `id` alone + // would discard a divergent object that merely reused the identifier. + if state.stack.back() != Some(command.entry.as_ref()) { + return Err(ResolvedStackPopReplayInvariantError::PoppedEntryMismatch); + } + + let entry = state + .stack + .pop_back() + .expect("the entry was just verified at the top of the stack"); + state.stack_paid_facts.remove(&entry.id); + state.stack_trigger_event_batches.remove(&entry.id); + Ok(()) +} + /// CR 603.3d: removes an uncommitted triggered ability from the stack. /// /// The "push first, choose second" invariant (see @@ -667,11 +758,14 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { state.announced_source_x = None; // CR 405.5: When all players pass in succession, the top object on the stack resolves. - let entry = match state.stack.pop_back() { - Some(e) => e, - None => return, + let Some(PoppedStackEntry { + entry, + paid_facts: paid_snapshot, + trigger_event_batch, + }) = pop_top_stack_entry(state) + else { + return; }; - let paid_snapshot = state.stack_paid_facts.remove(&entry.id); // CR 113.3b: Activated keyword abilities (Equip / Crew / Saddle / Station) // resolve via their typed payload — they have no ResolvedAbility/targets @@ -686,8 +780,6 @@ pub fn resolve_top(state: &mut GameState, events: &mut Vec) { return; } - let trigger_event_batch = state.stack_trigger_event_batches.remove(&entry.id); - // CR 603.4: Intervening-if condition rechecked at resolution time. if let StackEntryKind::TriggeredAbility { condition: Some(ref condition), @@ -3145,18 +3237,15 @@ fn resolve_batched( // CR 400.7j: clear the resolution-scoped self-move re-latch with the entry. state.resolution_source_relatch = None; - // Pop the run's entries (resolution order is back-to-front), cleaning the - // per-entry side tables exactly as `resolve_top` does for a single entry. + // Pop the run's entries (resolution order is back-to-front) through the same + // authority `resolve_top` uses for a single entry, so the per-entry side + // tables settle with each removal. let mut popped = Vec::with_capacity(consumed as usize); for _ in 0..consumed { - match state.stack.pop_back() { - Some(entry) => { - state.stack_paid_facts.remove(&entry.id); - state.stack_trigger_event_batches.remove(&entry.id); - popped.push(entry); - } - None => break, - } + let Some(removed) = pop_top_stack_entry(state) else { + break; + }; + popped.push(removed.entry); } // CR 603.7c: Set the trigger event context once from the (identical) top @@ -3482,13 +3571,11 @@ fn resolve_inert_noop_batch( // CR 400.7j: clear the resolution-scoped self-move re-latch with the entry. state.resolution_source_relatch = None; for _ in 0..consumed { - let Some(entry) = state.stack.pop_back() else { + let Some(removed) = pop_top_stack_entry(state) else { break; }; - state.stack_paid_facts.remove(&entry.id); - state.stack_trigger_event_batches.remove(&entry.id); events.push(GameEvent::StackResolved { - object_id: entry.id, + object_id: removed.entry.id, }); } consumed diff --git a/crates/engine/src/types/mod.rs b/crates/engine/src/types/mod.rs index 8297bbc24f..5708f14c58 100644 --- a/crates/engine/src/types/mod.rs +++ b/crates/engine/src/types/mod.rs @@ -92,8 +92,9 @@ pub use resolved_commands::{ ResolvedPlayerEditCommand, ResolvedPlayerEditReplayInvariantError, ResolvedRngReplayInvariantError, ResolvedRulesCommand, ResolvedRulesJournal, ResolvedRulesJournalError, ResolvedStackEntryFinalizeCommand, - ResolvedStackEntryFinalizeReplayInvariantError, ResolvedStackPushCommand, - ResolvedStackPushOrigin, ResolvedStackPushReplayInvariantError, ResolvedTriggerCollection, + ResolvedStackEntryFinalizeReplayInvariantError, ResolvedStackPopCommand, + ResolvedStackPopReplayInvariantError, ResolvedStackPushCommand, ResolvedStackPushOrigin, + ResolvedStackPushReplayInvariantError, ResolvedTriggerCollection, ResolvedTriggerCollectionCommand, ResolvedTriggerCollectionReplayInvariantError, ResolvedTriggerLedgerEdit, ResolvedUncommittedTriggerRemovalCommand, ResolvedUncommittedTriggerRemovalReplayInvariantError, RulesExecutionNodeKind, diff --git a/crates/engine/src/types/resolved_commands.rs b/crates/engine/src/types/resolved_commands.rs index 34017bc59e..4599ef4025 100644 --- a/crates/engine/src/types/resolved_commands.rs +++ b/crates/engine/src/types/resolved_commands.rs @@ -1151,6 +1151,47 @@ pub enum ResolvedUncommittedTriggerRemovalReplayInvariantError { UnexpectedRemovableEntry(ObjectId), } +/// One exact CR 405.2 removal of the topmost object from the stack. +/// +/// Recorded by `stack::pop_top_stack_entry`, the single authority behind the +/// CR 405.5 resolution pop and the drain loops (batched resolution, inert no-op +/// batches, CR 724.1b stack exile). A drain of N entries records N of these +/// rather than one bulk removal, so a replay reproduces the removal ORDER and +/// not merely the final depth. +/// +/// NOT unified with [`ResolvedUncommittedTriggerRemovalCommand`], despite both +/// removing a stack entry. The axis would have to span CR 603.3d +/// trigger-construction and CR 405.5 resolution — different rule sections the +/// engine resolves separately — and the operand sets genuinely differ: the +/// CR 603.3d removal consumes a cursor and may legitimately pop NOTHING, an +/// outcome that has no analogue here. Unifying them would buy one enum variant +/// at the cost of an applier that checks preconditions belonging to whichever +/// family it was not handed. +/// +/// The removed side-table VALUES are deliberately not recorded, for the same +/// reason the CR 603.3d removal omits them: this command installs nothing. It +/// drops rows keyed on the recorded entry's own id, so no recorded value would +/// pin an invariant, and carrying `Vec` batches would widen every +/// journal entry on the hottest path in the engine for nothing. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ResolvedStackPopCommand { + /// The removed entry, recorded verbatim so replay verifies the whole object + /// rather than trusting the id. + pub entry: Box, + /// Stack depth AFTER the removal (CR 405.2). + pub resulting_depth: usize, + pub cause: RulesExecutionNodeRef, +} + +/// Typed failure while applying one already-resolved CR 405.2 stack pop. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum ResolvedStackPopReplayInvariantError { + #[error("stack pop expected depth {expected} before removal, found {found}")] + DepthMismatch { expected: usize, found: usize }, + #[error("stack pop expected a different entry on top of the stack")] + PoppedEntryMismatch, +} + /// Semantic command payload currently carried by a resolved-rules journal entry. /// /// Additional command families are intentionally added by their owning P2 @@ -1181,6 +1222,7 @@ pub enum ResolvedRulesCommand { StackPush(Box), StackEntryFinalize(Box), UncommittedTriggerRemoval(Box), + StackPop(Box), } /// An append-only trigger collection command has no replay-time precondition. @@ -2258,6 +2300,17 @@ impl ResolvedRulesJournal { ) } + /// Records one exact CR 405.2 top-of-stack removal under its cause. + pub fn record_stack_pop( + &mut self, + command: ResolvedStackPopCommand, + ) -> Result { + self.append_command( + command.cause, + ResolvedRulesCommand::StackPop(Box::new(command)), + ) + } + fn begin_settlement( &mut self, identity_for: impl FnOnce(SettlementNodeOrdinal) -> RulesExecutionNodeRef, @@ -2474,7 +2527,8 @@ impl ResolvedRulesJournal { | ResolvedRulesCommand::TriggerCollection(_) | ResolvedRulesCommand::StackPush(_) | ResolvedRulesCommand::StackEntryFinalize(_) - | ResolvedRulesCommand::UncommittedTriggerRemoval(_) => {} + | ResolvedRulesCommand::UncommittedTriggerRemoval(_) + | ResolvedRulesCommand::StackPop(_) => {} } } for node in &self.nodes { @@ -2938,6 +2992,19 @@ impl ResolvedRulesJournal { } } } + ResolvedRulesCommand::StackPop(command) => { + // Cause-only. A pop draws no id and no timestamp, so there is no + // allocator receipt to cross-check. Both of its preconditions + // (the CR 405.2 depth and the exact entry on top) are + // state-dependent and are enforced by + // `stack::apply_resolved_stack_pop`, where the state exists to + // check them against. + if entry.node != command.cause { + return Err(ResolvedRulesJournalError::InvalidSerializedAuthority( + "stack pop command has an unrelated cause".to_string(), + )); + } + } } Ok(()) } diff --git a/crates/engine/tests/integration/cr733_resolved_commands_p2.rs b/crates/engine/tests/integration/cr733_resolved_commands_p2.rs index 8d33dd46a9..72763ae508 100644 --- a/crates/engine/tests/integration/cr733_resolved_commands_p2.rs +++ b/crates/engine/tests/integration/cr733_resolved_commands_p2.rs @@ -144,6 +144,9 @@ fn apply_semantic_command(state: &mut GameState, command: &ResolvedRulesCommand) ) .unwrap(); } + ResolvedRulesCommand::StackPop(command) => { + engine::game::stack::apply_resolved_stack_pop(state, command.as_ref()).unwrap(); + } } } @@ -247,9 +250,8 @@ fn exact_mana_spend_rejects_a_second_removal() { | ResolvedRulesCommand::TriggerCollection(_) | ResolvedRulesCommand::StackPush(_) | ResolvedRulesCommand::StackEntryFinalize(_) - | ResolvedRulesCommand::UncommittedTriggerRemoval(_) => { - apply_semantic_command(&mut replay, command) - } + | ResolvedRulesCommand::UncommittedTriggerRemoval(_) + | ResolvedRulesCommand::StackPop(_) => apply_semantic_command(&mut replay, command), } } assert!( diff --git a/crates/engine/tests/integration/cr733_resolved_draw.rs b/crates/engine/tests/integration/cr733_resolved_draw.rs index 8e8424b684..20a72809fa 100644 --- a/crates/engine/tests/integration/cr733_resolved_draw.rs +++ b/crates/engine/tests/integration/cr733_resolved_draw.rs @@ -109,6 +109,9 @@ fn apply_semantic_command(state: &mut GameState, command: &ResolvedRulesCommand) ) .unwrap(); } + ResolvedRulesCommand::StackPop(command) => { + engine::game::stack::apply_resolved_stack_pop(state, command.as_ref()).unwrap(); + } } } diff --git a/crates/engine/tests/integration/cr733_resolved_stack_pop.rs b/crates/engine/tests/integration/cr733_resolved_stack_pop.rs new file mode 100644 index 0000000000..efe23366d3 --- /dev/null +++ b/crates/engine/tests/integration/cr733_resolved_stack_pop.rs @@ -0,0 +1,332 @@ +//! CR733 P2 coverage for the CR 405.2 top-of-stack removal. +//! +//! CR 405.5: "When all players pass in succession, the top (last-added) object +//! on the stack resolves." That removal, plus the drain loops that clear several +//! entries in one pass (batched resolution, inert no-op batches, and the +//! CR 724.1b end-phase stack exile), all funnel through the single authority +//! `stack::pop_top_stack_entry`, which drops the entry together with the two +//! per-entry side tables keyed on it. +//! +//! A drain of N entries journals N separate commands rather than one bulk +//! removal, so a replay reproduces the removal ORDER and not merely the final +//! depth. `resolving_two_spells_journals_two_pops_in_lifo_order` is what pins +//! that: it would still pass on a bulk record if it only checked the end state, +//! so it asserts the per-command depths descend. +//! +//! THE CROSS-POP CANARY IS THE ACCEPTANCE TEST FOR THIS FAMILY. Before pops were +//! journaled, `apply_resolved_stack_push` failed `StackDepthMismatch` on any +//! replay whose prefix crossed a removal, and the push suite's module header +//! recorded that every replay there deliberately used a pop-free prefix. +//! `a_recorded_pop_unblocks_a_later_push_replay` flips exactly that: from ONE +//! predecessor it shows the push alone still failing and pop-then-push +//! succeeding. Do not weaken it to a bare success assertion — the failing half +//! is what proves the pop record is doing the work. + +use engine::game::scenario::{GameRunner, GameScenario, P0}; +use engine::game::stack::{apply_resolved_stack_pop, apply_resolved_stack_push}; +use engine::types::actions::GameAction; +use engine::types::game_state::{CastPaymentMode, GameState}; +use engine::types::identifiers::ObjectId; +use engine::types::mana::ManaCost; +use engine::types::phase::Phase; +use engine::types::resolved_commands::{ + ResolvedRulesCommand, ResolvedStackPopCommand, ResolvedStackPopReplayInvariantError, + ResolvedStackPushCommand, ResolvedStackPushReplayInvariantError, +}; + +/// A no-target sorcery: nothing to retarget, and no trigger to add stack entries +/// the pop assertions would have to filter around. +const ELVISH_TOKEN_SPELL: &str = "Create a 1/1 green Elf Warrior creature token."; + +/// Every stack pop journaled after `from`, in journal order. +fn stack_pops(state: &GameState, from: usize) -> Vec { + state + .resolved_rules_journal + .entries() + .iter() + .skip(from) + .filter_map(|entry| entry.command.clone()) + .filter_map(|command| match command { + ResolvedRulesCommand::StackPop(command) => Some(*command), + _ => None, + }) + .collect() +} + +/// Every stack push journaled after `from`, in journal order. +fn stack_pushes(state: &GameState, from: usize) -> Vec { + state + .resolved_rules_journal + .entries() + .iter() + .skip(from) + .filter_map(|entry| entry.command.clone()) + .filter_map(|command| match command { + ResolvedRulesCommand::StackPush(command) => Some(*command), + _ => None, + }) + .collect() +} + +fn cast(runner: &mut GameRunner, spell: ObjectId) { + let card_id = runner.state().objects[&spell].card_id; + runner + .act(GameAction::CastSpell { + object_id: spell, + card_id, + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }) + .expect("the real cast must put the spell on the stack"); +} + +fn scenario_with_spells(names: &[&str]) -> (GameRunner, Vec) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let ids = names + .iter() + .map(|name| { + scenario + .add_spell_to_hand_from_oracle(P0, name, true, ELVISH_TOKEN_SPELL) + .with_mana_cost(ManaCost::zero()) + .id() + }) + .collect(); + (scenario.build(), ids) +} + +#[test] +fn resolving_a_spell_journals_an_exact_pop() { + let (mut runner, ids) = scenario_with_spells(&["Journal Pop One"]); + let spell = ids[0]; + cast(&mut runner, spell); + + // Reach guard: the pop assertions below are meaningless unless the spell is + // genuinely on the stack first. + let before_pop = runner.state().clone(); + assert_eq!( + before_pop.stack.len(), + 1, + "CR 405.1: the cast spell is the only object on the stack" + ); + let journal_start = before_pop.resolved_rules_journal.entries().len(); + + runner.resolve_top(); + + // The discriminating assertion: the removal is journaled. A raw + // `stack.pop_back()` records nothing here. + let pops = stack_pops(runner.state(), journal_start); + let recorded: Vec<_> = pops.iter().filter(|pop| pop.entry.id == spell).collect(); + assert_eq!( + recorded.len(), + 1, + "CR 405.5: resolving the spell must journal exactly one pop for it" + ); + let pop = recorded[0]; + assert_eq!( + pop.resulting_depth, 0, + "CR 405.2: the recorded depth is the depth AFTER the removal" + ); + assert_eq!( + *pop.entry, before_pop.stack[0], + "the recorded entry is the entry that was on the stack, verbatim" + ); + + // Replay-exactness: from the captured predecessor, applying the record + // reproduces the removal with nothing re-derived. + let mut replay = before_pop.clone(); + apply_resolved_stack_pop(&mut replay, pop) + .expect("the recorded pop must replay against its captured predecessor"); + assert!(replay.stack.is_empty(), "replay removes the recorded entry"); + assert!( + !replay.stack_paid_facts.contains_key(&spell), + "replay drops the paid-facts row keyed on the removed entry" + ); + assert!( + !replay.stack_trigger_event_batches.contains_key(&spell), + "replay drops the trigger-batch row keyed on the removed entry" + ); + + // Re-applying is not idempotent: the stack is now shallower than recorded, + // so it fails closed rather than popping an unrelated object. + assert!( + matches!( + apply_resolved_stack_pop(&mut replay, pop), + Err(ResolvedStackPopReplayInvariantError::DepthMismatch { + expected: 1, + found: 0 + }) + ), + "a stack pop is not idempotent: a second application must fail closed" + ); +} + +/// Each probe diverges exactly one axis, so a rejection can only come from the +/// precondition being probed. +#[test] +fn pop_rejects_a_divergent_predecessor() { + let (mut runner, ids) = scenario_with_spells(&["Journal Pop Two"]); + let spell = ids[0]; + cast(&mut runner, spell); + let before_pop = runner.state().clone(); + let journal_start = before_pop.resolved_rules_journal.entries().len(); + runner.resolve_top(); + let pops = stack_pops(runner.state(), journal_start); + let pop = pops + .iter() + .find(|pop| pop.entry.id == spell) + .expect("the resolution journaled a pop"); + + // Right entry, wrong depth: a deeper stack means the replay is not at the + // point the record describes. + let mut too_deep = before_pop.clone(); + let mut duplicate = before_pop.stack[0].clone(); + duplicate.id = ObjectId(9999); + too_deep.stack.push_front(duplicate); + assert!( + matches!( + apply_resolved_stack_pop(&mut too_deep, pop), + Err(ResolvedStackPopReplayInvariantError::DepthMismatch { + expected: 1, + found: 2 + }) + ), + "a pop must refuse a predecessor at the wrong depth" + ); + assert_eq!( + too_deep.stack.len(), + 2, + "the rejected replay must not have mutated the stack" + ); + + // Right depth, wrong entry on top. Comparing the entry WHOLE rather than by + // id is what catches this — an applier matching on `id` alone would happily + // discard a divergent object that reused the identifier. + let mut wrong_top = before_pop.clone(); + wrong_top + .stack + .back_mut() + .expect("the predecessor has the entry on top") + .source_id = ObjectId(4242); + assert!( + matches!( + apply_resolved_stack_pop(&mut wrong_top, pop), + Err(ResolvedStackPopReplayInvariantError::PoppedEntryMismatch) + ), + "a pop must refuse a predecessor whose top entry diverges from the record" + ); + assert_eq!( + wrong_top.stack.len(), + 1, + "the rejected replay must not have mutated the stack" + ); +} + +/// A drain records one command per entry, in removal order — not one bulk +/// removal that only pins the final depth. +#[test] +fn resolving_two_spells_journals_two_pops_in_lifo_order() { + let (mut runner, ids) = scenario_with_spells(&["Journal Pop Lower", "Journal Pop Upper"]); + let (lower, upper) = (ids[0], ids[1]); + cast(&mut runner, lower); + cast(&mut runner, upper); + + // Reach guard: both entries are live, and `upper` is the one CR 405.5 will + // remove first. + let before = runner.state().clone(); + assert_eq!(before.stack.len(), 2, "both spells are on the stack"); + assert_eq!( + before.stack.back().map(|entry| entry.id), + Some(upper), + "CR 405.2: the last-added spell is on top" + ); + let journal_start = before.resolved_rules_journal.entries().len(); + + runner.advance_until_stack_empty(); + + let pops: Vec<_> = stack_pops(runner.state(), journal_start) + .into_iter() + .filter(|pop| pop.entry.id == lower || pop.entry.id == upper) + .collect(); + assert_eq!(pops.len(), 2, "each removal is journaled separately"); + assert_eq!( + (pops[0].entry.id, pops[1].entry.id), + (upper, lower), + "CR 405.5: the last-added object resolves first, and the records carry \ + that order" + ); + assert_eq!( + (pops[0].resulting_depth, pops[1].resulting_depth), + (1, 0), + "the recorded depths descend one per removal, which a single bulk record \ + could not express" + ); +} + +/// THE CANARY FLIP. See the module header: before this family, a replay whose +/// prefix crossed a pop failed `StackDepthMismatch` in the push applier. +#[test] +fn a_recorded_pop_unblocks_a_later_push_replay() { + let (mut runner, ids) = scenario_with_spells(&["Journal Pop First", "Journal Push Second"]); + let (first, second) = (ids[0], ids[1]); + + cast(&mut runner, first); + let predecessor = runner.state().clone(); + assert_eq!( + predecessor.stack.len(), + 1, + "reach guard: the replay predecessor has the first spell on the stack" + ); + let journal_start = predecessor.resolved_rules_journal.entries().len(); + + // Resolve the first spell (journals a pop), then cast the second (journals a + // push at the depth the pop left behind). + runner.resolve_top(); + cast(&mut runner, second); + + let pop = stack_pops(runner.state(), journal_start) + .into_iter() + .find(|pop| pop.entry.id == first) + .expect("resolving the first spell journaled its pop"); + let push = stack_pushes(runner.state(), journal_start) + .into_iter() + .find(|push| push.entry.id == second) + .expect("casting the second spell journaled its push"); + assert_eq!( + push.resulting_position, 0, + "CR 405.2: the second spell lands at index 0 precisely because the pop \ + emptied the stack first" + ); + + // BEFORE-half: the push alone cannot replay against this predecessor, + // because the un-removed first spell leaves the stack one deeper than the + // push recorded. This is the exact failure the push suite's header records. + let mut push_only = predecessor.clone(); + assert!( + matches!( + apply_resolved_stack_push(&mut push_only, &push), + Err(ResolvedStackPushReplayInvariantError::StackDepthMismatch { + expected: 0, + found: 1 + }) + ), + "the push record must still fail against a predecessor that has not had \ + the pop applied — otherwise this test proves nothing about the pop" + ); + + // AFTER-half: same predecessor, pop applied first, and the push now lands. + let mut sequenced = predecessor.clone(); + apply_resolved_stack_pop(&mut sequenced, &pop) + .expect("the recorded pop replays against the predecessor"); + apply_resolved_stack_push(&mut sequenced, &push) + .expect("with the pop applied, the push replays across it"); + assert_eq!( + sequenced.stack.len(), + 1, + "the sequenced replay leaves exactly the second spell on the stack" + ); + assert_eq!( + sequenced.stack[push.resulting_position], *push.entry, + "CR 405.2: the replay installs the recorded entry at the recorded index" + ); +} diff --git a/crates/engine/tests/integration/cr733_resolved_stack_push.rs b/crates/engine/tests/integration/cr733_resolved_stack_push.rs index 42128a4405..4a2337fd5e 100644 --- a/crates/engine/tests/integration/cr733_resolved_stack_push.rs +++ b/crates/engine/tests/integration/cr733_resolved_stack_push.rs @@ -17,11 +17,17 @@ //! point is unobservable. Do not replace it with a simpler card. //! //! Every replay below applies a recorded push to a prefix containing no stack -//! POP, because pops are not journaled yet (CR 608.1 / CR 603.3c/d / CR 701.6a -//! / CR 601.2a). Past the first un-journaled removal the recorded -//! `resulting_position` no longer matches the replayed depth and the applier -//! fails closed by design — so a future test that replays across a pop should -//! assert `StackDepthMismatch`, not relax the precondition. +//! POP, and the `StackDepthMismatch` assertions here are about a push applied +//! TWICE — a push is not idempotent — not about crossing a removal. +//! +//! The CR 405.2 top-of-stack pop IS journaled now +//! (`cr733_resolved_stack_pop.rs`), so a replay may cross one by applying the +//! recorded pop first; `a_recorded_pop_unblocks_a_later_push_replay` in that +//! file demonstrates exactly that against a predecessor where the push alone +//! still fails. Removals that remain un-journaled (remove-at-index and +//! retain-by-predicate, CR 701.6a and the CR 800.4a elimination sweep) still +//! make a crossing replay fail closed by design — the fix there is to journal +//! them, never to relax this applier's precondition. use engine::game::scenario::{GameRunner, GameScenario, P0}; use engine::game::stack::apply_resolved_stack_push; diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index a84a1fc1a9..83f6ee17cb 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -105,6 +105,7 @@ mod cr733_resolved_modifier_install; mod cr733_resolved_object_cease; mod cr733_resolved_player_leave; mod cr733_resolved_stack_entry_finalize; +mod cr733_resolved_stack_pop; mod cr733_resolved_stack_push; mod cr733_resolved_token_creation; mod cr733_resolved_transform;