diff --git a/crates/engine/src/game/effects/mod.rs b/crates/engine/src/game/effects/mod.rs index a3b006412d..2d8e6c27e9 100644 --- a/crates/engine/src/game/effects/mod.rs +++ b/crates/engine/src/game/effects/mod.rs @@ -889,6 +889,84 @@ fn clear_post_replacement_token_choice_seed_if_resolution_drained(state: &mut Ga } } +/// CR 603.3 + CR 603.3b + CR 608.2c: retire every ownerless `Dispatching` +/// post-replacement resident on the active frame. +/// +/// `Dispatching` means "taken and running". The sole production dispatcher — +/// `engine_replacement::apply_pending_post_replacement_effect` — is synchronous, +/// and `engine_replacement::post_replacement_dispatch_is_live` reports whether +/// any such dispatch is on this thread's call stack. When that predicate is +/// false, a `Dispatching` entry has no owner AT ALL: `begin_dispatch` refuses +/// it, `finish_paused_dispatch` pops only `Paused`, and `finish_dispatch` needs +/// a handle that died with its call frame. Left behind it keeps +/// `resolution_stack` non-empty forever, which makes +/// `triggers::resolution_completion_can_settle` false forever: deferred +/// triggered abilities can then never be put on the stack the next time a +/// player would receive priority (CR 603.3 + CR 603.3b) and the resolving +/// carrier can never settle (CR 608.2c). +/// +/// The predicate is sound only because the dispatcher's cleanup leaves every +/// returned dispatch either `Paused` (parked awaiting a player answer, CR +/// 614.12a) or removed. A parked continuation therefore never presents as +/// `Dispatching`, which is what the `h1_devour_...` row witnesses at an action +/// boundary. +/// +/// TWO CALL SITES, answering two different questions — this is deliberate, and +/// neither is redundant: +/// * `resume_resolution_frames` (below), at a priority boundary, for a strand +/// THIS action just created. CR 603.3 requires the abilities it blocks to +/// reach the stack the next time a player would receive priority in this +/// same action, not one action later. +/// * `engine::apply_action_boundary_core`, at the ENTRY of the outer action +/// boundary, for a strand the engine FOUND rather than made — above all one +/// that arrived through `PersistedGameState::into_game_state()`, which no +/// in-action seam can observe because no engine write produced it. +/// +/// Because the ownerless predicate is call-stack state rather than game state, +/// it is equally valid at both, at any frame depth and any drain depth. +/// +/// The loop handles a multi-entry strand (both drains in the turn-20 capture); +/// it stops at the first `Ready` or `Paused` resident, which is live parked work. +/// +/// RECOVERY LIMIT, stated rather than overclaimed: this reaches only the frame +/// the two-deep positional accessor can see. A wedge whose `PostReplacement` +/// frame is buried under a child frame that itself never drains is NOT +/// recovered here; only identity-addressed dispatch prevents that shape. +/// +/// This emits no `GameEvent`: removing an impossible state is not a game event. +pub(crate) fn sweep_ownerless_post_replacement_strand(state: &mut GameState) { + if crate::game::engine_replacement::post_replacement_dispatch_is_live() { + // A live dispatch legitimately suppresses the sweep. In healthy play + // this branch is unreachable: this policy is entered only from + // `resume_resolution_frames` at a priority boundary and from + // `engine::apply_action_boundary_core` at an outer action boundary, + // never from inside a dispatch. It fires on every boundary forever if + // the guard flag has leaked, which is the WASM `panic='abort'` residual + // documented on `POST_REPLACEMENT_DISPATCH_LIVE` — this warn is what + // makes that observable instead of silent. It is visible only in the + // server-hosted native build; `engine-wasm` installs no `tracing` + // subscriber, and this does NOT close that gap. + if state + .active_post_replacement_drains() + .and_then(crate::types::game_state::PostReplacementDrainStack::resident) + .is_some_and(|drain| { + matches!( + drain.status, + crate::types::game_state::DrainStatus::Dispatching + ) + }) + { + tracing::warn!("post-replacement sweep suppressed by a live dispatch"); + } + } else if let Some(drains) = state.active_post_replacement_drains_mut() { + while drains.finish_ownerless_dispatching_resident().is_some() { + tracing::warn!( + "retired an ownerless Dispatching post-replacement drain at a rest boundary" + ); + } + } +} + /// Resume the active typed resolution frame through its runtime stack authority. /// /// The dispatcher reads only the stack top. The one shipped coupled shape is @@ -1009,6 +1087,7 @@ pub(crate) fn resume_resolution_frames(state: &mut GameState, events: &mut Vec = + const { std::cell::Cell::new(false) }; +} + +/// RAII: sets the flag, restores the PREVIOUS value on drop — nesting-correct, +/// so the CR 616.1g nested same-frame dispatch (Jace/Swans) keeps it set for +/// the outer dispatch after the inner one returns. +#[must_use] +struct LiveDispatchGuard(bool); + +impl LiveDispatchGuard { + fn enter() -> Self { + LiveDispatchGuard(POST_REPLACEMENT_DISPATCH_LIVE.with(|flag| flag.replace(true))) + } +} + +impl Drop for LiveDispatchGuard { + fn drop(&mut self) { + POST_REPLACEMENT_DISPATCH_LIVE.with(|flag| flag.set(self.0)); + } +} + +/// True while any `apply_pending_post_replacement_effect` dispatch is between +/// its `begin_dispatch` and its cleanup on this thread. The priority-boundary +/// sweeper consults this before retiring a `Dispatching` resident: when it is +/// false, "ownerless" is a definition rather than an inference, at any frame +/// depth and at any drain depth. +pub(crate) fn post_replacement_dispatch_is_live() -> bool { + POST_REPLACEMENT_DISPATCH_LIVE.with(std::cell::Cell::get) +} + pub(crate) fn apply_pending_post_replacement_effect( state: &mut GameState, object_id: Option, @@ -2537,9 +2590,14 @@ pub(crate) fn apply_pending_post_replacement_effect( .map(|drain| std::mem::take(&mut drain.applied)) .unwrap_or_default(); - let (continuation, dispatch) = state - .active_post_replacement_drains_mut()? - .begin_dispatch()?; + let (continuation, dispatch) = state.begin_post_replacement_dispatch()?; + // CR 603.3b + CR 608.2c: this dispatch is now live on this thread's call + // stack. Constructed AFTER the mint so the `?` early-return path never + // enters it, and dropped by RAII at every exit — the + // `capture_deferred_entry_events_if_mid_entry_choice` tail, every `?`, and + // (on unwind profiles) a panic. While it is held, the priority-boundary + // sweeper must not treat a `Dispatching` resident as ownerless. + let _live_dispatch = LiveDispatchGuard::enter(); let waiting_for = match continuation { PostReplacementContinuation::Resolved(resolved) => { apply_post_replacement_resolved_effect(state, &resolved, replacement_applied, events) @@ -2554,21 +2612,42 @@ pub(crate) fn apply_pending_post_replacement_effect( events, ), }; - // CR 615.5: a direct pause retains this dispatch's context on its own entry. - // If a nested drain owns the prompt instead, this continuation has completed; - // retire its exact `Dispatching` entry below the nested top. - if waiting_for.is_some() - && state - .active_post_replacement_drains() - .is_some_and(|drains| drains.dispatch_is_resident_top(dispatch)) - { - let _ = state - .active_post_replacement_drains_mut() - .is_some_and(|drains| drains.pause_dispatch(dispatch)); + // CR 614.12a + CR 616.1g: classify by what OWNS the outstanding prompt, never + // by whether the two-deep positional accessor can currently see this frame. + // The predecessor asked `active_post_replacement_drains()`, which returns None + // as soon as this frame's DISTANCE FROM THE STACK TOP exceeds one — whether + // because the dispatched continuation raised two or more frames, or because a + // parent-of-active insert slid a frame in above it + // (`ResolutionStack::insert_parent_of_active`, reached from + // `effects::append_to_pending_continuation`). Note the frame need not have + // moved at all: `active_post_replacement_parent_index` looks only at the top + // and its immediate predecessor, by design. Both arms then degraded to no-ops + // and the entry was stranded `Dispatching` — unrecoverable, because every + // retirement path is keyed on `Ready`, on `Paused`, or on a handle that dies + // with this call. Worse, on the same miss + // `GameState::install_post_replacement_drain` mints a SIBLING PostReplacement + // frame, and the accessor then hands this cleanup that frame's drain vector — + // so the outer `finish_dispatch` could remove an unrelated frame's entry. + // The dispatch now names its frame by a stable `PostReplacementFrameId`, bound + // to the frame at most once, so both failure modes are closed: the lookup + // follows the frame through every `frames.insert` / `frames.swap`, never + // resolves to a sibling, and is not invalidated by a nested same-frame + // dispatch. + if waiting_for.is_some() && state.post_replacement_dispatch_is_resident_top(dispatch) { + // This dispatch's own work owns the prompt: park it so the paused-retire + // paths (`finish_active_paused_post_replacement_dispatch` and the + // `resume_resolution_frames` Paused branch) can finish it after the answer. + let _ = state.pause_post_replacement_dispatch(dispatch); } else { - let _ = state - .active_post_replacement_drains_mut() - .and_then(|drains| drains.finish_dispatch(dispatch)); + // Either the continuation completed, or a NESTED drain above this entry + // owns the prompt (CR 616.1g) — in both cases this entry's work is done. + // Retire exactly `dispatch`, addressed by frame id, wherever that frame + // now sits. + let _ = state.finish_post_replacement_dispatch(dispatch); + // Unchanged from the predecessor, and deliberately NOT folded into the + // call above: this removes any empty PostReplacement frame that is now the + // stack top, which is a wider set than "the frame this dispatch + // addressed". Narrowing it would be a second, unrelated behavioural delta. state.remove_empty_active_post_replacement_frame(); } // NOTE: the inherited token-choice applied seed is intentionally NOT cleared @@ -7415,4 +7494,158 @@ mod tests { other => panic!("expected CopyTargetChoice, got {other:?}"), } } + + /// Park a state on Priority whose single resolution frame is a + /// `PostReplacement` holding one `Dispatching` resident — the wedge shape, + /// reached the only way it can be reached without a live dispatcher: an + /// install followed by a direct `begin_dispatch` whose handle is then + /// dropped, exactly as a real dispatcher's handle dies with its call frame. + fn state_with_ownerless_dispatching_resident() -> GameState { + let mut state = GameState::new_two_player(42); + state.waiting_for = WaitingFor::Priority { + player: PlayerId(0), + }; + state.priority_player = PlayerId(0); + + let mut drains = crate::types::game_state::PostReplacementDrainStack::default(); + assert!( + drains.install( + crate::types::game_state::PostReplacementDrain::ready( + PostReplacementContinuation::Resolved(Box::new(ResolvedAbility::new( + Effect::Draw { + count: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Controller, + }, + Vec::new(), + ObjectId(81), + PlayerId(0), + ))), + ), + crate::types::game_state::ResidentDrainPolicy::KeepResident, + ), + "the fixture's ready drain installs" + ); + let (_continuation, _handle) = drains + .begin_dispatch() + .expect("a ready resident begins dispatching"); + state.resolution_stack.push_post_replacement(drains); + state + } + + fn resident_is_dispatching(state: &GameState) -> bool { + state + .active_post_replacement_drains() + .and_then(crate::types::game_state::PostReplacementDrainStack::resident) + .is_some_and(|drain| { + matches!( + drain.status, + crate::types::game_state::DrainStatus::Dispatching + ) + }) + } + + /// **H5 — discriminating(U1), two halves.** A `Dispatching` entry whose + /// dispatcher is still on this thread's call stack is live work, not a + /// strand, and the priority-boundary sweeper must leave it alone. + /// + /// Carried WITHOUT a CR citation, deliberately. This is an invariant of the + /// engine's own dispatch machinery — which in-flight work the sweeper may + /// retire — and no rule of the game speaks to it. It formerly cited + /// CR 615.5, which says a prevention effect may include an additional effect + /// referring to the amount of damage that was prevented, the rest of the + /// effect taking place immediately after the prevention itself. + /// + /// That rule is real authority for the PREVENTION family's additional-effect + /// reference, and it is why `begin_dispatch` + /// (`types/game_state.rs`) and `apply_pending_post_replacement_effect` above + /// both still cite it: the engine satisfies that reference by keeping the + /// drain resident with its event context readable, which is how + /// `PostReplacementSourceController` resolves Swans of Bryn Argoll. What + /// CR 615.5 does NOT speak to is dispatch LIVENESS — whether a `Dispatching` + /// entry is still on this thread's call stack — and that is this test's + /// actual subject. So the number is dropped HERE rather than swapped for a + /// guess. Do not read that omission as a reason to strip CR 615.5 from the + /// event-context sites, where it is the correct anchor. + /// + /// Half (i) holds a `LiveDispatchGuard` across the sweep and asserts the + /// drain SURVIVES. Hand-patching `post_replacement_dispatch_is_live()` to + /// return `false` turns half (i) red behaviourally — the sweep fires + /// mid-dispatch and destroys the event context the running continuation + /// reads. + /// + /// Half (ii) repeats the sweep with no guard held and asserts the drain IS + /// retired. It is half (i)'s positive control: without it, half (i) would be + /// satisfied by a sweep that never fires at all. Reverting the sweep turns + /// half (ii) red. + /// + /// Half (iii) tests the shared policy FUNCTION rather than one of its callers. + /// The guard is the only protection the new entry-side call site in + /// `engine::apply_action_boundary_core` has, so it is asserted at the function + /// both callers share. Half (iii)'s second call is its own positive control: + /// without it, a policy that never sweeps anything would pass. + /// + /// `LiveDispatchGuard::enter()` stays module-private deliberately — the + /// guard's soundness argument is "the dispatcher is the sole thing that can + /// make a dispatch live", and a `pub(crate) enter()` would downgrade that + /// from a compiler-enforced fact to a convention. + #[test] + fn h5_a_live_dispatch_suppresses_the_ownerless_sweep_and_its_absence_permits_it() { + // Half (i): a live dispatch is on this thread's call stack. + let mut state = state_with_ownerless_dispatching_resident(); + assert!( + resident_is_dispatching(&state), + "reach-guard: the fixture parks a Dispatching resident" + ); + let mut events = Vec::new(); + { + let _live = LiveDispatchGuard::enter(); + assert!( + post_replacement_dispatch_is_live(), + "reach-guard: the guard actually marks the dispatch live" + ); + crate::game::effects::resume_resolution_frames(&mut state, &mut events); + } + assert!( + resident_is_dispatching(&state), + "a live dispatch's resident must survive the priority-boundary sweep" + ); + assert_eq!( + state.resolution_stack.len(), + 1, + "the frame that owns a live dispatch must survive too" + ); + + // Half (ii), the positive control: with no dispatch live, the same call + // retires the same entry — so half (i) is not passing on an inert sweep. + assert!( + !post_replacement_dispatch_is_live(), + "the guard restored the previous value on drop" + ); + crate::game::effects::resume_resolution_frames(&mut state, &mut events); + assert!( + state.resolution_stack.is_empty(), + "CR 603.3b + CR 608.2c: an ownerless Dispatching resident and its emptied frame retire" + ); + + // Half (iii): the shared policy FUNCTION, called directly, is guard + // suppressed exactly as its `resume_resolution_frames` caller is. + let mut direct = state_with_ownerless_dispatching_resident(); + assert!( + resident_is_dispatching(&direct), + "reach-guard: the fixture parks a Dispatching resident" + ); + { + let _live = LiveDispatchGuard::enter(); + crate::game::effects::sweep_ownerless_post_replacement_strand(&mut direct); + } + assert!( + resident_is_dispatching(&direct), + "a live dispatch's resident survives the shared policy function too" + ); + crate::game::effects::sweep_ownerless_post_replacement_strand(&mut direct); + assert!( + !resident_is_dispatching(&direct), + "half (iii)'s own positive control: with no dispatch live the same direct call retires it" + ); + } } diff --git a/crates/engine/src/game/triggers_devour_runtime_tests.rs b/crates/engine/src/game/triggers_devour_runtime_tests.rs index a213a48212..1e1a74657f 100644 --- a/crates/engine/src/game/triggers_devour_runtime_tests.rs +++ b/crates/engine/src/game/triggers_devour_runtime_tests.rs @@ -674,3 +674,339 @@ fn devour_food_3_sacrifices_only_food_subtype() { "Devour Food 3 + one Food → 3 +1/+1 counters" ); } + +// --------------------------------------------------------------------------- +// Post-replacement drain-strand rows. The dispatcher these exercise is shared by +// every as-enters replacement continuation, not by Devour alone; Devour is the +// cheapest production driver for it that needs no card data. +// --------------------------------------------------------------------------- + +/// Place a permanent under `controller` carrying the triggered abilities parsed +/// from `oracle_text`, and index them so they can fire. Verbatim Oracle text is +/// used rather than a hand-built `TriggerDefinition` so the fixture cannot take a +/// different parser branch than the real card. +fn battlefield_trigger_observer( + state: &mut GameState, + controller: PlayerId, + name: &str, + oracle_text: &str, +) -> ObjectId { + let card_id = CardId(state.next_object_id); + let id = create_object( + state, + card_id, + controller, + name.to_string(), + Zone::Battlefield, + ); + let parsed = crate::parser::oracle::parse_oracle_text( + oracle_text, + name, + &[], + &["Enchantment".to_string()], + &[], + ); + assert!( + !parsed.triggers.is_empty(), + "the observer fixture must parse at least one trigger from {oracle_text:?}" + ); + let mut face = CardFace { + name: name.to_string(), + triggers: parsed.triggers, + ..CardFace::default() + }; + face.card_type.core_types.push(CoreType::Enchantment); + { + let obj = state.objects.get_mut(&id).unwrap(); + apply_card_face_to_object(obj, &face); + } + crate::game::trigger_index::reindex_object_triggers(state, id); + id +} + +/// True when any `PostReplacement` frame in the stack still holds a +/// `Dispatching` drain. Read back through the stack's own `Serialize` impl +/// because `PostReplacementDrainStack` exposes only its resident, and a strand +/// can sit below a nested entry. +fn has_dispatching_drain(state: &GameState) -> bool { + let value = + serde_json::to_value(&state.resolution_stack).expect("the resolution stack serializes"); + value["frames"].as_array().is_some_and(|frames| { + frames.iter().any(|frame| { + frame["type"] == "PostReplacement" + && frame["data"]["drains"].as_array().is_some_and(|drains| { + drains.iter().any(|drain| drain["status"] == "Dispatching") + }) + }) + }) +} + +/// The per-`PostReplacement`-frame drain statuses of the runtime state, read +/// back through the stack's own `Serialize` impl. `PostReplacementDrainStack` +/// exposes only its resident, so this is how a test observes WHICH status a +/// parked entry carries — and a multi-entry strand — without widening +/// production API for a test's convenience. A verbatim port of the shipped +/// helper of the same name in +/// `crates/engine/tests/integration/mycoloth_devour_drain_strand.rs`. +fn post_replacement_drain_statuses(state: &GameState) -> Vec> { + let value = + serde_json::to_value(&state.resolution_stack).expect("the resolution stack serializes"); + value["frames"] + .as_array() + .expect("frames is an array") + .iter() + .filter(|frame| frame["type"] == "PostReplacement") + .map(|frame| { + frame["data"]["drains"] + .as_array() + .expect("a post-replacement frame carries a drains array") + .iter() + .map(|drain| match &drain["status"] { + serde_json::Value::String(status) => status.clone(), + // `DrainStatus::Ready(_)` is externally tagged. + serde_json::Value::Object(map) => { + map.keys().next().cloned().unwrap_or_default() + } + other => other.to_string(), + }) + .collect() + }) + .collect() +} + +/// **B2-devour — guard (relabelled).** The Devour producer path leaves no +/// `Dispatching` drain behind. +/// +/// This row DISCRIMINATES NOTHING and must not be read as evidence for the +/// producer analysis. It was measured on a tree with U1 and U2 both absent and +/// reported no strand at all, so it is green on `main` and must stay green under +/// every discrimination patch in the plan's §5.8. It is kept as an honest +/// regression witness for the reporter's own card class — Devour is what the +/// reporter played — not as a red-capable row. +/// +/// The discriminating producer row is +/// `b2_zurs_weirding_replacement_leaves_no_dispatching_drain` in +/// `tests/integration/mycoloth_devour_drain_strand.rs`, built on the Zur's +/// Weirding draw-replacement path, which was MEASURED to strand at BASE_SHA. +/// +/// CR 702.82a + CR 614.12a + CR 603.3b. Positive reach-guards, so no negative +/// here can be satisfied vacuously: the prompt was actually raised, the counters +/// landed (`p1p1 == 2 * n_sacrificed`), and both sacrificed creatures reached the +/// graveyard so their observers really did fire. +#[test] +fn b2_devour_producer_path_leaves_no_dispatching_drain() { + let face = devour_face("Mycoloth", 2); + let mut fodder = Vec::new(); + let (mut state, devour) = drive_devour_etb_with_battlefield(&face, PlayerId(0), |state| { + fodder.push(battlefield_creature(state, PlayerId(0), "Sac Fodder 0")); + fodder.push(battlefield_creature(state, PlayerId(0), "Sac Fodder 1")); + battlefield_trigger_observer( + state, + PlayerId(0), + "Bastion of Remembrance", + "Whenever a creature you control dies, each opponent loses 1 life and you gain 1 life.", + ); + battlefield_trigger_observer( + state, + PlayerId(0), + "Sacrifice Ledger", + "Whenever you sacrifice a creature, draw a card.", + ); + }); + + let WaitingFor::EffectZoneChoice { cards, .. } = &state.waiting_for else { + panic!( + "reach-guard: the Devour sacrifice prompt must be raised, got {:?}", + state.waiting_for + ); + }; + let chosen: Vec = fodder + .iter() + .copied() + .filter(|id| cards.contains(id)) + .collect(); + assert_eq!(chosen.len(), 2, "both fodder creatures are eligible"); + + crate::game::engine::apply_as_current( + &mut state, + GameAction::SelectCards { + cards: chosen.clone(), + }, + ) + .unwrap(); + + // Positive reach-guards: the path really resolved. + assert_eq!( + p1p1(&state, devour), + 4, + "Devour 2 x 2 sacrifices → 4 +1/+1 counters (CR 702.82a)" + ); + for id in &chosen { + assert_eq!( + state.objects.get(id).unwrap().zone, + Zone::Graveyard, + "each sacrificed creature reached the graveyard, so its observers fired" + ); + } + + assert!( + !has_dispatching_drain(&state), + "no post-replacement drain may survive its own dispatch as Dispatching" + ); +} + +/// **H1 — guard.** *Hostile: the empty/decline path.* CR 702.82a: a Devour +/// entry with ZERO creatures sacrificed still installs a continuation and still +/// retires it, leaving no resolution frame behind. +/// +/// Positive reach-guard: the prompt was genuinely raised before the empty +/// submission, so `resolution_stack.is_empty()` cannot pass because nothing ever +/// installed a drain. +/// +/// **H7 — guard, the non-interference witness for the ENTRY call site.** The +/// submission below routes `apply_as_current -> apply_as_current_with_mode -> +/// apply_action_boundary -> apply_action_boundary_with_stack_limit -> +/// apply_action_boundary_core`, so the entry-side sweep added at that function +/// runs on THIS state — a healthy one, parked mid-prompt with a live owner. +/// This row is therefore also the row that ESTABLISHES the premise §5.4's +/// safety rests on: parked-awaiting-input work carries `Paused`, never +/// `Dispatching` (CR 614.12a — the as-enters choice is still being made, so the +/// drain is live rules work). The pre-submit `post_replacement_drain_statuses` +/// assertion is the witness; the post-submit assertions are the survival proof. +#[test] +fn h1_devour_empty_sacrifice_retires_its_drain_and_frame() { + let face = devour_face("Gorger Wurm", 1); + let (mut state, devour) = drive_devour_etb_to_sacrifice_choice(&face, PlayerId(0), 2); + + assert!( + matches!(state.waiting_for, WaitingFor::EffectZoneChoice { .. }), + "reach-guard: a drain was installed and its prompt raised, got {:?}", + state.waiting_for + ); + + // NON-INTERFERENCE WITNESS for the entry-side sweep added at + // `engine::apply_action_boundary_core`. The submission below routes + // `apply_as_current -> apply_as_current_with_mode -> apply_action_boundary + // -> apply_action_boundary_with_stack_limit -> apply_action_boundary_core`, + // so the entry sweep runs on THIS state — a healthy one, parked mid-prompt + // with a live owner. CR 614.12a: the as-enters choice is still being made, + // so the drain is live parked work and MUST survive the boundary. Its + // resident is `Paused`, never `Dispatching`, which is exactly the status the + // sweep's exhaustive match refuses to pop — and it is what makes the + // "Dispatching + no live dispatch => ownerless" predicate sound. A sweep + // written with a wildcard arm, or scoped to `!Paused`, destroys it here and + // reds the completion assertions below. + assert_eq!( + post_replacement_drain_statuses(&state), + vec![vec!["Paused".to_string()]], + "reach-guard: exactly one parked post-replacement drain, and it must be live parked \ + work (`Paused`) so the entry sweep provably runs over a healthy resident" + ); + + crate::game::engine::apply_as_current(&mut state, GameAction::SelectCards { cards: vec![] }) + .unwrap(); + + assert_eq!( + p1p1(&state, devour), + 0, + "an empty Devour sacrifice places 0 counters (CR 702.82a)" + ); + assert!( + !has_dispatching_drain(&state), + "the declined continuation must not leave a Dispatching drain" + ); + assert!( + state.resolution_stack.is_empty(), + "the decline branch leaves no resolution frame behind, got {:?}", + state.resolution_stack.len() + ); +} + +/// **H4 — guard.** *Hostile: source/controller change.* CR 603.3b: a dies +/// observer controlled by a DIFFERENT player than the devourer's controller +/// still reaches the stack when the devoured creatures die. +/// +/// Positive reach-guard: the observer's controller is asserted to differ from +/// the devourer's controller, and the sacrificed creatures are asserted to have +/// reached the graveyard — so "the trigger fired" is not satisfiable by an +/// observer that never saw a death. +#[test] +fn h4_foreign_controller_dies_observer_still_reaches_the_stack() { + let face = devour_face("Mycoloth", 2); + let mut fodder = Vec::new(); + let mut observer = ObjectId(0); + let (mut state, devour) = drive_devour_etb_with_battlefield(&face, PlayerId(0), |state| { + fodder.push(battlefield_creature(state, PlayerId(0), "Sac Fodder 0")); + fodder.push(battlefield_creature(state, PlayerId(0), "Sac Fodder 1")); + observer = battlefield_trigger_observer( + state, + PlayerId(1), + "Foreign Mourner", + "Whenever a creature dies, you gain 1 life.", + ); + }); + + assert_ne!( + state.objects[&observer].controller, state.objects[&devour].controller, + "reach-guard: the observer must be controlled by the OTHER player" + ); + + let WaitingFor::EffectZoneChoice { cards, .. } = &state.waiting_for else { + panic!( + "reach-guard: the Devour sacrifice prompt must be raised, got {:?}", + state.waiting_for + ); + }; + let chosen: Vec = fodder + .iter() + .copied() + .filter(|id| cards.contains(id)) + .collect(); + assert_eq!(chosen.len(), 2, "both fodder creatures are eligible"); + + crate::game::engine::apply_as_current( + &mut state, + GameAction::SelectCards { + cards: chosen.clone(), + }, + ) + .unwrap(); + + for id in &chosen { + assert_eq!( + state.objects.get(id).unwrap().zone, + Zone::Graveyard, + "reach-guard: the observer really did see a creature die" + ); + } + + // Pin WHICH destination, not merely that one of them holds. A disjunction over + // {stack, deferred, pending order} is satisfied by a trigger that fired and then + // parked forever — the exact failure this change repairs — so it cannot witness + // CR 603.3b for a guard row whose claim is arrival. `assert_eq!` on the observed + // destination reports the actual one when it moves. + let destination = if state.stack.iter().any(|entry| entry.source_id == observer) { + "stack" + } else if state + .deferred_triggers + .iter() + .any(|deferred| deferred.pending.source_id == observer) + { + "deferred" + } else if state.pending_trigger_order.as_ref().is_some_and(|order| { + order.groups.iter().any(|group| { + group + .triggers + .iter() + .any(|pending| pending.pending.source_id == observer) + }) + }) { + "pending_order" + } else { + "nowhere" + }; + assert_eq!( + destination, "stack", + "CR 603.3b: the foreign-controller dies trigger must reach the stack" + ); +} diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index f1e2efbb72..26cce430a9 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -17501,6 +17501,15 @@ pub enum DrainStatus { /// the synchronous dispatcher finish or pause the entry it took after a nested /// replacement has pushed another drain above it. It is never serialized and /// introduces no cross-carrier reference. +/// +/// It addresses an entry *within* one drain stack, and says nothing about WHICH +/// frame that stack belongs to. `types::resolution::IdentifiedPostReplacementDispatch` +/// is what pairs it with the identity of that frame, so a cleanup can find the +/// frame wherever the stack has since moved it. This type is deliberately left +/// unchanged by that pairing: widening it would change +/// [`PostReplacementDrainStack::begin_dispatch`]'s signature, which every +/// existing call site — including two bare-path `Option::and_then` uses in +/// `game/elimination.rs` — depends on. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct PostReplacementDrainDispatch { depth: usize, @@ -17640,6 +17649,14 @@ pub enum ResidentDrainPolicy { #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] pub struct PostReplacementDrainStack { drains: Vec, + /// This frame's stable identity, bound at its first dispatch and never + /// rebound. Private, and reached only through [`Self::frame_id`] / + /// [`Self::stamp_frame_id`], so "assigned at most once" is a property of the + /// type rather than of any call site. `skip_serializing_if` keeps an + /// unstamped frame's wire shape byte-identical to what it was before this + /// field existed. + #[serde(default, skip_serializing_if = "Option::is_none")] + id: Option, } impl PostReplacementDrain { @@ -17815,10 +17832,75 @@ impl PostReplacementDrainStack { None } + /// CR 603.3b + CR 608.2c: retire a resident whose dispatch is over. + /// + /// `Dispatching` means "taken and running" (see [`DrainStatus`]). The sole + /// production dispatcher — `engine_replacement::apply_pending_post_replacement_effect` + /// — is synchronous, and `engine_replacement::post_replacement_dispatch_is_live` + /// reports whether any such dispatch is on this thread's call stack. The + /// sweeper calls this ONLY when that predicate is false, so a `Dispatching` + /// entry reaching here has no owner at all: `begin_dispatch` refuses it, + /// `finish_paused_dispatch` pops only `Paused`, and `finish_dispatch` needs a + /// handle that died with its call frame. Left behind, it keeps + /// `resolution_stack` non-empty forever, which makes + /// `triggers::resolution_completion_can_settle` false forever: deferred + /// triggered abilities can then never be put on the stack (CR 603.3b) and the + /// resolving carrier can never settle (CR 608.2c). + /// + /// Scope is the RESIDENT only, and that is exact rather than conservative. A + /// `Dispatching` entry sitting BELOW a nested `Paused` one is never the + /// resident, so it is never reached here; it is retired later, after the + /// `Paused` entry above it retires and it becomes the resident at a boundary + /// where — again — no dispatch is live. A `Dispatching` entry below another + /// `Dispatching` one is reachable only after the caller's loop has popped the + /// one above it, and the ownerless predicate is stack-wide rather than + /// per-entry, so it is equally ownerless. A `Ready` or `Paused` resident is + /// live parked work and is returned untouched. + pub fn finish_ownerless_dispatching_resident(&mut self) -> Option { + match self.drains.last()?.status { + DrainStatus::Dispatching => self.drains.pop(), + DrainStatus::Ready(_) | DrainStatus::Paused => None, + } + } + /// CR 800.4a: abandon every pending continuation (player departure). pub fn abandon_all(&mut self) { self.drains.clear(); } + + /// This frame's identity, or `None` if it has never been dispatched. + /// + /// "Unstamped" is carried by the type, not by a reserved value: a legacy + /// payload, a freshly minted sibling frame, and a journal-replayed frame are + /// all `None`, and `None` can never equal a handle's `Some(id)`. + pub(super) fn frame_id(&self) -> Option { + self.id + } + + /// Bind identity exactly once, and return the EFFECTIVE id. + /// + /// If this frame is already stamped, `candidate` is discarded and the + /// existing id is returned. That is not an optimisation — it is the + /// invariant. A nested same-frame dispatch is the shipped CR 616.1g shape (a + /// running continuation draws, the draw is replaced, and the replacement + /// carries a mandatory post-effect — Jace, Wielder of Mysteries' win; see + /// [`Self::install`]'s doc). Re-stamping there would invalidate the OUTER + /// dispatch's still-live handle and strand its entry `Dispatching` forever — + /// the exact failure this identity exists to close. Keeping the rule inside + /// the type means no call site can violate it. The caller detects "was my + /// candidate consumed?" by comparing the returned id against the candidate, + /// and only then commits its allocator, so no id is burned. + /// + /// This writes ONLY the frame's own id. It deliberately does not touch + /// `ResolutionStack::last_post_replacement_frame_id`, which is the mint's + /// business — that asymmetry is what lets a test build a payload whose + /// frames carry ids while its allocator is still 0. + pub(super) fn stamp_frame_id( + &mut self, + candidate: PostReplacementFrameId, + ) -> PostReplacementFrameId { + *self.id.get_or_insert(candidate) + } } /// Legacy pre-`DrawSequenceStack` save shape: the single in-flight multi-card @@ -17840,6 +17922,25 @@ pub struct PendingMultiDraw { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] pub struct DrawSequenceFrameId(pub u64); +/// Identifies one `PostReplacement` frame within a [`ResolutionStack`]. +/// +/// Frames are addressed by ID, never by position, for the same reason +/// [`DrawSequenceFrameId`] is: between a dispatch's mint and its cleanup the +/// frame may have left the two-deep positional window the accessor can see — +/// because a nested instruction (CR 616.1g) raised child frames above it, or +/// because a parent-of-active continuation insert slid a frame in between. In +/// neither case did the frame itself move; only its distance from the stack top +/// changed, and that is the only thing positional addressing can observe. +/// +/// Deliberately no `Default`: the field that holds one is +/// `Option`, whose `Default` is `None` regardless of +/// `T`, so "unstamped" is carried by the type rather than by a reserved zero id +/// that could alias a real frame. +/// +/// [`ResolutionStack`]: crate::types::resolution::ResolutionStack +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] +pub struct PostReplacementFrameId(pub u64); + /// The unbookkept suffix of one individual draw whose Library → Hand delivery /// parked on a replacement choice. /// @@ -19520,6 +19621,54 @@ impl GameState { Ok(completed) } + /// CR 614.12a + CR 616.1g: take the active frame's resident continuation and + /// return it paired with the identity of the frame it came from. + /// + /// Pure delegation to [`ResolutionStack::begin_active_post_replacement_dispatch`], + /// which is where the frame index, the id stamp and the allocator commit all + /// live. This exists so the dispatcher keeps talking to `GameState`. + /// + /// [`ResolutionStack::begin_active_post_replacement_dispatch`]: super::resolution::ResolutionStack::begin_active_post_replacement_dispatch + pub(crate) fn begin_post_replacement_dispatch( + &mut self, + ) -> Option<( + crate::types::ability::PostReplacementContinuation, + super::resolution::IdentifiedPostReplacementDispatch, + )> { + self.resolution_stack + .begin_active_post_replacement_dispatch() + } + + /// Whether `dispatch` still owns the resident top of its OWN frame, found by + /// identity rather than by position. Pure delegation. + pub(crate) fn post_replacement_dispatch_is_resident_top( + &self, + dispatch: super::resolution::IdentifiedPostReplacementDispatch, + ) -> bool { + self.resolution_stack + .post_replacement_dispatch_is_resident_top(dispatch) + } + + /// Park `dispatch`'s exact entry in its own frame. Pure delegation. + pub(crate) fn pause_post_replacement_dispatch( + &mut self, + dispatch: super::resolution::IdentifiedPostReplacementDispatch, + ) -> bool { + self.resolution_stack + .pause_post_replacement_dispatch(dispatch) + } + + /// Retire `dispatch`'s exact entry in its own frame. Pure delegation — it + /// removes no frame; `remove_empty_active_post_replacement_frame` keeps that + /// job, and keeps its wider "any empty frame that is now the top" scope. + pub(crate) fn finish_post_replacement_dispatch( + &mut self, + dispatch: super::resolution::IdentifiedPostReplacementDispatch, + ) -> Option { + self.resolution_stack + .finish_post_replacement_dispatch(dispatch) + } + /// Retires only the exact top general drain whose continuation paused and /// whose MultiDraw child has already completed. pub fn finish_active_paused_post_replacement_dispatch(&mut self) { @@ -24081,6 +24230,308 @@ mod drain_stack_reentrancy_tests { dispatching drain (CR 616.1g), not dropped" ); } + + // ----------------------------------------------------------------------- + // Identity-addressed dispatch rows. + // ----------------------------------------------------------------------- + + fn resident_status(state: &GameState) -> Option<&DrainStatus> { + match state.resolution_stack.last() { + Some(crate::types::resolution::ResolutionFrame::PostReplacement(drains)) => { + drains.drains.first().map(|drain| &drain.status) + } + _ => None, + } + .or_else(|| { + state.resolution_stack.iter().find_map(|frame| match frame { + crate::types::resolution::ResolutionFrame::PostReplacement(drains) => { + drains.drains.first().map(|drain| &drain.status) + } + _ => None, + }) + }) + } + + /// **B1 — discriminating(U2).** CR 614.12a + CR 616.1g: a dispatch whose own + /// continuation BURIED its frame still addresses its exact entry. + /// + /// The continuation is an `Effect::Discard` carrying a sub-ability. Parking + /// on `DiscardChoice` raises a direct-choice frame, and the sub-ability then + /// takes `append_to_pending_continuation`'s parent-of-active branch, which + /// inserts an `AbilityContinuation` BETWEEN the `PostReplacement` frame and + /// the active child. The frame's absolute index never changes; its distance + /// from the top goes from 1 to 2, which is the only thing the two-deep + /// positional accessor can see. + /// + /// Revert-failing assertion: the entry reads `Paused`. With the cleanup + /// resolved positionally the accessor misses, both arms degrade to no-ops, + /// and the entry is left `Dispatching` — the strand. + #[test] + fn b1_a_dispatch_whose_continuation_buried_its_frame_still_parks_its_own_entry() { + let mut state = GameState::new_two_player(42); + state.waiting_for = WaitingFor::Priority { + player: PlayerId(0), + }; + state.priority_player = PlayerId(0); + for index in 0..3u64 { + let id = crate::game::zones::create_object( + &mut state, + crate::types::identifiers::CardId(500 + index), + PlayerId(0), + format!("Hand Card {index}"), + crate::types::zones::Zone::Hand, + ); + let _ = id; + } + + let mut discard_with_tail = AbilityDefinition::new( + AbilityKind::Spell, + Effect::Discard { + count: crate::types::ability::QuantityExpr::Fixed { value: 1 }, + target: crate::types::ability::TargetFilter::Controller, + selection: crate::types::ability::CardSelectionMode::Chosen, + unless_filter: None, + filter: None, + }, + ); + discard_with_tail = discard_with_tail.sub_ability( + AbilityDefinition::new( + AbilityKind::Spell, + Effect::GainLife { + amount: crate::types::ability::QuantityExpr::Fixed { value: 1 }, + player: crate::types::ability::TargetFilter::Controller, + }, + ) + // CR 701.9a + CR 608.2c: a discard whose tail is gated on what was + // discarded raises a real `ResolutionFrame::Discard` for the operation + // (`effects/discard.rs::resolve` mints it on the PRESENCE of this + // condition variant, not on its contents). The parked tail then becomes a + // second frame above it, so the `PostReplacement` frame sits two deep and + // the two-deep positional accessor can no longer see it — which is the + // burial this row exists to measure. + .condition( + crate::types::ability::AbilityCondition::DiscardedCardMatchesFilter { + filter: crate::types::ability::TargetFilter::Any, + }, + ), + ); + + let mut drains = PostReplacementDrainStack::default(); + assert!(drains.install( + PostReplacementDrain::ready(PostReplacementContinuation::Template(Box::new( + discard_with_tail + ))), + ResidentDrainPolicy::KeepResident, + )); + state.resolution_stack.push_post_replacement(drains); + + // Reach-guard: the mint really handed out a pair — this row is not + // passing because no dispatch ever started. + assert!( + state + .active_post_replacement_drains() + .is_some_and(PostReplacementDrainStack::has_ready), + "the fixture parks a Ready drain reachable by the mint" + ); + + let mut events = Vec::new(); + let waiting = crate::game::engine_replacement::apply_pending_post_replacement_effect( + &mut state, + None, + None, + None, + &mut events, + ); + + // Reach-guard: the continuation ran AND buried its own frame. + assert!( + waiting.is_some(), + "the continuation must park on a prompt, got {:?}", + state.waiting_for + ); + assert!( + state.resolution_stack.len() >= 3, + "the continuation must have raised two frames above its own, got {}", + state.resolution_stack.len() + ); + assert!( + state.active_post_replacement_drains().is_none(), + "reach-guard: the two-deep positional accessor can no longer see the frame — \ + without this the row measures nothing" + ); + + assert!( + matches!(resident_status(&state), Some(DrainStatus::Paused)), + "CR 614.12a: the dispatch parks its OWN entry wherever its frame now sits; \ + a positional cleanup no-ops and leaves it Dispatching. got {:?}", + resident_status(&state) + ); + } + + /// **H2 — guard.** *Hostile: the negative sibling status.* A resident + /// `Ready` drain reached at a priority boundary must still be DISPATCHED, + /// never swept. + /// + /// This is `main`'s behaviour with no sweep at all, so it stays green under + /// a sweep revert — that is why it is a guard. Its assertion is + /// mutation-failing: a sweep written with a wildcard arm, or scoped to + /// `!Paused` rather than to `Dispatching` alone, swallows the `Ready` + /// continuation and turns this red. + /// + /// Positive control: the drain's effect is observed to have executed, so + /// "the drain is gone" cannot be satisfied by a sweep that discarded it. + #[test] + fn h2_a_ready_resident_is_dispatched_at_a_priority_boundary_never_swept() { + let mut state = GameState::new_two_player(42); + state.waiting_for = WaitingFor::Priority { + player: PlayerId(0), + }; + state.priority_player = PlayerId(0); + let life_before = state.players[0].life; + + let mut drains = PostReplacementDrainStack::default(); + assert!(drains.install( + PostReplacementDrain::ready(PostReplacementContinuation::Template(Box::new( + AbilityDefinition::new( + AbilityKind::Spell, + Effect::GainLife { + amount: crate::types::ability::QuantityExpr::Fixed { value: 3 }, + player: crate::types::ability::TargetFilter::Controller, + }, + ) + ))), + ResidentDrainPolicy::KeepResident, + )); + state.resolution_stack.push_post_replacement(drains); + + let mut events = Vec::new(); + crate::game::effects::resume_resolution_frames(&mut state, &mut events); + + assert_eq!( + state.players[0].life, + life_before + 3, + "positive control: the Ready continuation actually RAN — without this, \ + 'the drain is gone' would also be satisfied by a sweep that ate it" + ); + assert!( + state.resolution_stack.is_empty(), + "the dispatched-and-finished drain leaves no frame behind" + ); + } + + /// **H3 — discriminating(U2).** *Hostile: multi-authority CR 616.1g + /// nesting.* One frame, an outer `Dispatching` entry below an inner `Paused` + /// one, with the frame buried two deep. + /// + /// Three claims: + /// (i) the second (inner) mint against the same frame returns a pair whose + /// `frame()` EQUALS the outer's — red under an overwriting stamp; + /// (ii) the outer dispatch's finish removes ONLY depth 0; the inner `Paused` + /// entry survives with its own `event_source` intact and the frame's id + /// unchanged — red under a positional lookup; + /// (iii) the ownerless sweep does not fire here, because the resident is + /// `Paused`, which is live parked work. + #[test] + fn h3_nested_same_frame_dispatches_share_one_identity_and_retire_lifo() { + let mut stack = crate::types::resolution::ResolutionStack::default(); + stack.push_post_replacement(PostReplacementDrainStack::default()); + + let mut outer = ready_drain("outer"); + outer.event_source = Some(ObjectId(7)); + assert!(stack + .active_post_replacement_or_paired_parent_mut() + .expect("frame is active") + .install(outer, ResidentDrainPolicy::KeepResident)); + let (_, handle_outer) = stack + .begin_active_post_replacement_dispatch() + .expect("the outer ready drain begins dispatching"); + + let mut inner = ready_drain("inner"); + inner.event_source = Some(ObjectId(9)); + assert!(stack + .active_post_replacement_or_paired_parent_mut() + .expect("frame is active") + .install(inner, ResidentDrainPolicy::KeepResident)); + + // Positive control: two entries with DISTINCT event contexts exist. + match stack.last() { + Some(crate::types::resolution::ResolutionFrame::PostReplacement(drains)) => { + assert_eq!(drains.drains.len(), 2, "outer + inner both resident"); + assert_eq!(drains.drains[0].event_source, Some(ObjectId(7))); + assert_eq!(drains.drains[1].event_source, Some(ObjectId(9))); + assert!( + drains.frame_id().is_some(), + "the outer mint stamped the frame" + ); + } + other => panic!("expected the post-replacement frame, got {other:?}"), + } + + let (_, handle_inner) = stack + .begin_active_post_replacement_dispatch() + .expect("the inner ready drain begins dispatching"); + + // (i) The re-stamp regression witness. + assert_eq!( + handle_inner.frame(), + handle_outer.frame(), + "CR 616.1g: a nested same-frame dispatch reuses the frame's identity; \ + re-stamping invalidates the outer dispatch's still-live handle" + ); + assert!(stack.pause_post_replacement_dispatch(handle_inner)); + + // Bury the frame two deep, out of the two-deep positional window. + stack.push_inner(crate::types::resolution::ResolutionFrame::PostReplacement( + PostReplacementDrainStack::default(), + )); + stack.push_inner(crate::types::resolution::ResolutionFrame::PostReplacement( + PostReplacementDrainStack::default(), + )); + let frames_before = stack.len(); + + // (ii) The outer finish retires exactly depth 0. + let retired = stack.finish_post_replacement_dispatch(handle_outer); + assert!( + retired.is_some_and(|drain| drain.event_source == Some(ObjectId(7))), + "the outer dispatch retires its OWN entry, identified by event context" + ); + assert_eq!( + stack.len(), + frames_before, + "a buried frame is never removed by a finish — that would reorder the stack" + ); + // Bound out as a statement so the `iter()` temporary (which holds the borrow + // of `stack`) drops here rather than after `stack` itself — a tail-expression + // `match stack.iter().next()` outlives its own receiver. + let bottom_frame = stack.iter().next(); + match bottom_frame { + Some(crate::types::resolution::ResolutionFrame::PostReplacement(drains)) => { + assert_eq!(drains.drains.len(), 1, "only the inner entry survives"); + assert_eq!( + drains.drains[0].event_source, + Some(ObjectId(9)), + "CR 615.5: the surviving inner entry keeps its own prevented-event context" + ); + assert!( + matches!(drains.drains[0].status, DrainStatus::Paused), + "the inner entry is still parked" + ); + assert_eq!( + drains.frame_id(), + Some(handle_outer.frame()), + "the frame's identity is unchanged by either dispatch" + ); + + // (iii) The ownerless sweep does not fire on a Paused resident. + let mut probe = drains.clone(); + assert!( + probe.finish_ownerless_dispatching_resident().is_none(), + "a Paused resident is live parked work, never an ownerless strand" + ); + } + other => panic!("expected the original post-replacement frame, got {other:?}"), + } + } } #[cfg(test)] @@ -26765,6 +27216,18 @@ mod tests { /// intentionally outside the state-equality key just as it was before the /// ChangeZone frame migration. Replacing `game_state_eq` with derived stack /// equality makes this assertion fail. + /// + /// Exposure note for the per-frame `PostReplacementFrameId`: it DOES + /// participate in `GameState` equality, through `ResolutionStack::game_state_eq`'s + /// derived fall-through arm for `PostReplacement` — exactly the treatment + /// `DiscardFrame.id` already receives, and the direction that function's own + /// comment calls fail-safe (COMPARED is fail-safe; EXCLUSION is the + /// fail-DANGEROUS direction). Worst case: a repeating position that mints a + /// new frame id each iteration stops comparing equal, so the CR 104.4b + /// auto-pass window terminates on its iteration cap instead of on loop + /// detection. The ALLOCATOR is not exposed here at all — `game_state_eq` + /// compares `frames` only, the same treatment `next_draw_sequence_frame_id` + /// and `next_discard_frame_id` already get. #[test] fn game_state_equality_excludes_devour_only_change_zone_frame() { let state = GameState::new_two_player(7); diff --git a/crates/engine/src/types/resolution.rs b/crates/engine/src/types/resolution.rs index 211bef18d2..4f5c5d3a42 100644 --- a/crates/engine/src/types/resolution.rs +++ b/crates/engine/src/types/resolution.rs @@ -19,7 +19,8 @@ use crate::types::game_state::{ PendingEachPlayerCopyChosen, PendingLifeTotalAssignment, PendingMultiDraw, PendingPerCategoryZoneChoice, PendingPerPlayerZoneChoice, PendingRepeatIteration, PendingRepeatUntil, PendingSpellResolution, PendingVoteBallotIteration, PostReplacementDrain, - PostReplacementDrainStack, ResidentDrainPolicy, ResolvingTriggerContext, WaitingFor, + PostReplacementDrainDispatch, PostReplacementDrainStack, PostReplacementFrameId, + ResidentDrainPolicy, ResolvingTriggerContext, WaitingFor, }; use crate::types::identifiers::{DiscardFrameId, ObjectId}; use crate::types::player::PlayerId; @@ -448,6 +449,71 @@ pub struct ResolutionStack { /// rewinds, so a stale replacement event cannot bind to a later discard. #[serde(default)] next_discard_frame_id: u64, + /// The LAST post-replacement frame id allocated in this stack — named for + /// what it holds, not for the next value, because the two sibling + /// allocators above are *next* allocators and conflating the conventions is + /// what makes an off-by-one in `validate` look correct. + /// + /// Monotonic WITHIN AN ACTION, so a stale captured id cannot alias a later + /// frame. It is NOT monotonic across actions: every failure path in + /// `engine::apply_action_boundary_core` restores the whole state with + /// `*state = boundary_snapshot`, which rewinds this counter along with the + /// `frames` it numbered. + /// + /// The two sibling allocators above are NOT exempt from that rewind. All + /// three are fields of this one `ResolutionStack`, `GameState` holds exactly + /// one of those as `resolution_stack`, and `*state = boundary_snapshot` is a + /// wholesale `GameState` assignment — so all three rewind identically. The + /// siblings' flat "never rewinds" wording is scoped the same way this field's + /// is, to within an action, and has simply not been corrected yet. Do not + /// read it onto this field as though it stated a contrast, and do not take it + /// as license to make this counter survive a rollback. + /// + /// The rewind is safe precisely BECAUSE it is not selective: a handle is + /// unserialized and dies inside its synchronous dispatch, so every id this + /// counter issued during the rolled-back action was already dead by the time + /// the action failed, and the frames those ids named are rolled back in the + /// same assignment. Reissuing a rolled-back id can therefore only ever + /// re-number a frame that no live handle refers to. An allocator that + /// survived the rollback while its frames did not would be the actual + /// hazard, and is what this note exists to stop a future reader from + /// "fixing" the field into. + /// + /// Allocation is pre-increment, so the first id in any stack is 1 and + /// 0 is never a live id — but correctness does not rest on that: an + /// unstamped frame is `None`, and no handle can carry `None`. + #[serde(default)] + last_post_replacement_frame_id: u64, +} + +/// A dispatch handle bound to the identity of the frame that issued it. +/// +/// [`PostReplacementDrainDispatch`] addresses an entry WITHIN a drain stack; +/// this pairs it with the [`PostReplacementFrameId`] of the frame that drain +/// stack belongs to, so the dispatcher's cleanup can find that frame wherever it +/// has since moved to (CR 616.1g nesting, continuation inserts). Both fields are +/// private and neither is serialized: the pair must not outlive its synchronous +/// dispatch, and persisting it would legitimise exactly the cross-round-trip +/// `Dispatching` status this fix exists to forbid. +/// +/// Construction is confined to this module — there is deliberately no public +/// constructor and no `dispatch()` accessor. Note that in-module code, INCLUDING +/// this file's `#[cfg(test)] mod`, can still write a struct literal; unit rows +/// must nonetheless obtain every handle from +/// [`ResolutionStack::begin_active_post_replacement_dispatch`], because a +/// hand-built literal bypasses the stamp and the allocator commit and would go +/// green for the wrong reason. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct IdentifiedPostReplacementDispatch { + frame: PostReplacementFrameId, + dispatch: PostReplacementDrainDispatch, +} + +impl IdentifiedPostReplacementDispatch { + /// The identity of the frame this dispatch belongs to. + pub(crate) fn frame(self) -> PostReplacementFrameId { + self.frame + } } impl ResolutionStack { @@ -486,6 +552,14 @@ impl ResolutionStack { self.next_discard_frame_id = next_frame_id; } + pub(crate) fn last_post_replacement_frame_id(&self) -> u64 { + self.last_post_replacement_frame_id + } + + pub(crate) fn restore_last_post_replacement_frame_id(&mut self, last_frame_id: u64) { + self.last_post_replacement_frame_id = last_frame_id; + } + /// Starts one discard operation and returns its unique provenance id. pub fn begin_discard(&mut self, source_id: Option) -> DiscardFrameId { let id = DiscardFrameId(self.next_discard_frame_id); @@ -538,6 +612,30 @@ impl ResolutionStack { } } + /// Raise the post-replacement allocator above every id the restored frames + /// already carry, so the next mint cannot collide with a persisted frame and + /// so [`Self::validate`] accepts a payload whose outer allocator field was + /// absent. + /// + /// That is its ONLY job, stated exactly. It does not provide the `>= 1` + /// floor — allocation is pre-increment, so a fresh stack yields 1 without + /// any recovery — and it does not provide uniqueness against unstamped + /// frames, which are `None` and unaddressable by type. + fn recover_post_replacement_frame_id_allocator(&mut self) { + let last_frame_id = self + .frames + .iter() + .filter_map(|frame| match frame { + ResolutionFrame::PostReplacement(drains) => drains.frame_id().map(|id| id.0), + _ => None, + }) + .max(); + if let Some(last_frame_id) = last_frame_id { + self.last_post_replacement_frame_id = + self.last_post_replacement_frame_id.max(last_frame_id); + } + } + /// Compares runtime frames with the `GameState` equality contract. /// /// A Devour-only ChangeZone frame preserves a live CR 614.12a/614.13a @@ -2293,6 +2391,216 @@ impl ResolutionStack { .then_some(parent_index) } + /// CR 614.12a + CR 616.1g: take the active frame's resident continuation and + /// bind the dispatch to that frame's stable identity. + /// + /// Ordering here is load-bearing: + /// + /// * the frame index is resolved through the EXISTING private + /// [`Self::active_post_replacement_parent_index`], which keeps its + /// documented "no general frame search" contract and remains the + /// mint-time authority — identity addressing is added for the CLEANUP, + /// which is the only side that can be reached after the frame has moved; + /// * `begin_dispatch()` runs BEFORE any stamping, so a declined dispatch + /// (a `Dispatching` or `Paused` resident) consumes no id; + /// * the id is bound to the frame at most once, by `stamp_frame_id`, so a + /// nested same-frame dispatch reuses it and the outer handle stays valid; + /// * the allocator is committed only when the candidate was the id actually + /// taken, so no id is ever burned. + /// + /// The resulting id is strictly greater than every id already allocated in + /// this stack and therefore names exactly one frame — on a fresh game, with + /// no restore and no recovery pass. Legacy and unstamped frames are `None` + /// and are unaddressable by type. + pub(crate) fn begin_active_post_replacement_dispatch( + &mut self, + ) -> Option<( + crate::types::ability::PostReplacementContinuation, + IdentifiedPostReplacementDispatch, + )> { + let index = self.active_post_replacement_parent_index()?; + let candidate = + PostReplacementFrameId(self.last_post_replacement_frame_id.saturating_add(1)); + let Some(ResolutionFrame::PostReplacement(drains)) = self.frames.get_mut(index) else { + unreachable!("checked post-replacement parent must match") + }; + let (continuation, dispatch) = drains.begin_dispatch()?; + let frame = drains.stamp_frame_id(candidate); + // Commit the allocation only if this frame actually took the candidate. A + // nested same-frame dispatch (CR 616.1g) reuses the existing id and leaves + // the allocator untouched, and a declined `begin_dispatch` returns above + // without touching it either, so no id is ever burned. The equality test is + // exact rather than heuristic: every stamped id in this stack is + // <= `last_post_replacement_frame_id` (enforced by `validate` and by this + // rule), so a pre-existing id can never equal `last + 1`. + if frame == candidate { + self.last_post_replacement_frame_id = candidate.0; + } + Some(( + continuation, + IdentifiedPostReplacementDispatch { frame, dispatch }, + )) + } + + /// The SINGLE identity-addressed search over `frames`, and the only place in + /// this module licensed to search the frame vector at all. + /// + /// `scripts/check-resolution-frame-boundaries.sh` anchors its exemption to + /// this function BY NAME. The rule the guard enforces is "one search, here", + /// not "any search that looks identity-shaped": a second search anywhere in + /// this file — including a verbatim copy of the expression below, and + /// including one added INSIDE this body — still fails the guard, which + /// counts the searches in this span and requires exactly one. The guard + /// also requires that search to select on `frame_id() == Some(id)`, so the + /// exemption cannot be inherited by a positional probe that merely takes + /// this function's name. Move this function and the guard fails loudly + /// rather than silently widening. + /// + /// The distinction the guard draws is between POSITIONAL or + /// adjacency-inferred access, which guesses a structural relationship the + /// stack does not guarantee, and identity-addressed access, which asserts + /// one. The latter is an established access mode in this codebase, not a + /// carve-out invented here: [`DrawSequenceStack`]'s `frame_mut` / `active_if` + /// / `pop` address their frames the same way, resting on the same + /// monotonic-allocator property — an id is never reissued, so a stale id + /// matches nothing rather than aliasing a later frame. Unstamped frames + /// carry `None` and can never match. + /// + /// Both halves of that soundness argument are pinned by existing rows rather + /// than asserted here: `h6a_legacy_id_less_post_replacement_frames_restore_unstamped` + /// for the unstamped case, and + /// `v2_reader_recovers_discard_allocator_and_rejects_duplicate_frame_ids` + /// for the no-reissue case. If either is deleted, this exemption loses its + /// basis. + /// + /// [`DrawSequenceStack`]: crate::types::game_state::DrawSequenceStack + fn post_replacement_frame_index(&self, id: PostReplacementFrameId) -> Option { + self.frames.iter().position(|frame| { + matches!(frame, ResolutionFrame::PostReplacement(drains) if drains.frame_id() == Some(id)) + }) + } + + /// The drain stack of the frame `id` names, if that frame is still resident. + /// + /// The index does NOT escape this accessor pair. Handing a position to + /// callers would reintroduce exactly the positional coupling this change + /// exists to remove, which is why the three operations below take the + /// payload rather than an index. + fn post_replacement_frame( + &self, + id: PostReplacementFrameId, + ) -> Option<&PostReplacementDrainStack> { + let index = self.post_replacement_frame_index(id)?; + match self.frames.get(index) { + Some(ResolutionFrame::PostReplacement(drains)) => Some(drains), + Some(_) | None => { + unreachable!("the index came from a matched post-replacement frame") + } + } + } + + /// The mutable twin of [`Self::post_replacement_frame`]. + fn post_replacement_frame_mut( + &mut self, + id: PostReplacementFrameId, + ) -> Option<&mut PostReplacementDrainStack> { + let index = self.post_replacement_frame_index(id)?; + match self.frames.get_mut(index) { + Some(ResolutionFrame::PostReplacement(drains)) => Some(drains), + Some(_) | None => { + unreachable!("the index came from a matched post-replacement frame") + } + } + } + + /// Report a lookup that found no frame. The frame was consumed by + /// `GameState::abandon_active_replacement_tails`, so the operation is a + /// correct no-op — reported rather than silent. + /// + /// That is the ONLY consuming path named here because it is the only one + /// reachable during a live dispatch. `take_active_post_replacement` is this + /// module's shared primitive, not itself such a path: of its three callers, + /// `effects::resume_resolution_frames` reaches it only after + /// `apply_pending_post_replacement_effect` has returned, and + /// `GameState::remove_empty_active_post_replacement_frame` is gated on + /// `PostReplacementDrainStack::is_empty`, which counts the still-resident + /// `Dispatching` entry and so cannot be true of a live frame. The third is + /// `abandon_active_replacement_tails` itself, which is why the body below + /// names it alone. + /// + /// Reported by `warn!` ALONE, deliberately. This is NOT a proven-impossible + /// state, so a `debug_assert!(false)` here would panic every dev and test + /// build on a path that may be legitimate. Only the dispatcher's own live + /// handle can reach this (a handle is unserialized and dies inside its + /// synchronous dispatch), so entering it requires the frame to be consumed + /// DURING that dispatch — and the consuming path is not guarded against + /// that: `GameState::abandon_active_replacement_tails` pops the top + /// `PostReplacement` frame unconditionally, with no + /// `engine_replacement::post_replacement_dispatch_is_live` check of the kind + /// `effects::sweep_ownerless_post_replacement_strand` carries, and player + /// elimination (CR 800.4a) reaches it synchronously from a continuation's + /// own ability chain through `effects::win_lose` (CR 104.2b + CR 104.3e). + /// What is unsettled is only whether the `state.pending_replacement` + /// is-some gate on both `elimination.rs` call sites can still hold at that + /// instant. No test in the suite covers the path either way, so its absence + /// from a green run is not evidence of unreachability. + /// + /// The predecessor returned `None` here silently, so asserting would be a + /// newly minted panic rather than a preserved invariant. `warn!` is also + /// what survives the shipped WASM release profile, which is where this + /// class of bug lives. + fn report_missing_post_replacement_frame(dispatch: IdentifiedPostReplacementDispatch) { + tracing::warn!( + frame = dispatch.frame().0, + "post-replacement dispatch addressed a frame that is no longer on the resolution stack" + ); + } + + /// Whether `dispatch` still owns the resident top of its OWN frame. + pub(crate) fn post_replacement_dispatch_is_resident_top( + &self, + dispatch: IdentifiedPostReplacementDispatch, + ) -> bool { + let Some(drains) = self.post_replacement_frame(dispatch.frame) else { + Self::report_missing_post_replacement_frame(dispatch); + return false; + }; + drains.dispatch_is_resident_top(dispatch.dispatch) + } + + /// Park `dispatch`'s exact entry within its own frame. + pub(crate) fn pause_post_replacement_dispatch( + &mut self, + dispatch: IdentifiedPostReplacementDispatch, + ) -> bool { + let Some(drains) = self.post_replacement_frame_mut(dispatch.frame) else { + Self::report_missing_post_replacement_frame(dispatch); + return false; + }; + drains.pause_dispatch(dispatch.dispatch) + } + + /// Retire `dispatch`'s exact entry within its own frame. + /// + /// PURE delegation: it removes no frame. Folding frame removal in here would + /// silently narrow the shipped `GameState::remove_empty_active_post_replacement_frame` + /// from "any empty `PostReplacement` frame that is now the stack top" to + /// "the frame this dispatch addressed", which is a second, unrelated + /// behavioural delta. A buried empty frame is deliberately left in place — + /// removing a frame from under a live child reorders the stack — and is + /// removed by the existing `is_empty` block in the priority-boundary sweeper + /// once its children pop and it becomes the top. + pub(crate) fn finish_post_replacement_dispatch( + &mut self, + dispatch: IdentifiedPostReplacementDispatch, + ) -> Option { + let Some(drains) = self.post_replacement_frame_mut(dispatch.frame) else { + Self::report_missing_post_replacement_frame(dispatch); + return None; + }; + drains.finish_dispatch(dispatch.dispatch) + } + /// Returns the active ChangeZone frame, or its exact immediate parent /// while a post-replacement child raised by that zone change is active. /// This is the one Devour snapshot relationship that survives a paused @@ -2644,6 +2952,7 @@ impl ResolutionStack { } let has_multi_draw = multi_draw_count == 1; let mut discard_ids = HashSet::new(); + let mut post_replacement_ids = HashSet::new(); let mut direct_choice_count = 0; let mut buried_direct_choice = None; for (index, frame) in self.frames.iter().enumerate() { @@ -2684,6 +2993,32 @@ impl ResolutionStack { }); } } + if let ResolutionFrame::PostReplacement(drains) = frame { + // An unstamped frame is legal in any number: legacy payloads, + // freshly minted sibling frames and journal-replayed frames all + // carry `None`, and `None` can never alias a handle's `Some(id)`. + if let Some(id) = drains.frame_id() { + if !post_replacement_ids.insert(id) { + return Err(ResolutionStackError::InvalidPayload { + frame: FrameKind::PostReplacement, + message: "duplicate post-replacement frame id".to_string(), + }); + } + // `>`, NOT `>=`. The Discard and MultiDraw blocks compare + // `>=` because theirs are NEXT allocators whose valid range + // is `0..next`; this is a LAST-allocated allocator whose + // valid range is `1..=last`, so `>=` would reject every + // legal payload — including the one this stack just wrote. + if id.0 > self.last_post_replacement_frame_id { + return Err(ResolutionStackError::InvalidPayload { + frame: FrameKind::PostReplacement, + message: + "the resolution-stack post-replacement allocator is behind its active frame" + .to_string(), + }); + } + } + } if has_multi_draw && matches!( frame, @@ -2701,9 +3036,53 @@ impl ResolutionStack { "a paused post-replacement drain has no immediate multi-draw child", ))?; validate_shipped_post_replacement_draw_pair(frame, child)?; - if index + 2 != self.frames.len() { + // CR 614.11a + CR 121.6b: when a replacement replaces a draw + // inside a draw sequence, ALL actions the replacement requires + // are completed before the sequence resumes. If one of those + // actions is a player's choice, the game necessarily rests on + // that choice with the draw sequence still parked beneath it — + // so the paired child is NOT always the stack top. + // + // For a SINGLE-card draw — which is what the shipped Zur's + // Weirding rows drive (`DebugAction::DrawCards { count: 1 }`) — + // the ordering basis is CR 614.6 + CR 608.2c: the draw never + // happens and a modified event occurs instead, whose + // instructions are followed in the order written, so the "may + // pay 2 life" offer is carried out before the bin-or-draw tail. + // CR 614.11a covers the multi-draw sequence this same arm also + // guards (CR 121.2: a multi-card draw is that many individual + // card draws). + // + // The one frame that may sit above it is the frame that owns + // that choice: `FrameGate::DirectChoice(_)`, the prompt-owning + // family. No separate citation is carried here: the CR 614.11a / + // CR 614.6 / CR 608.2c basis stated one paragraph above is what + // makes a mid-application choice ordinary and is the whole + // authority for this admission. + // + // The admission is bounded four ways, three of them by rules + // this function already enforces: exactly one frame may sit + // above the pair (checked here); it must be a direct-prompt + // owner (checked here); it must be the ONLY direct-choice owner + // (`MultipleDirectChoiceOwners`, below); and its gate must match + // the live `waiting_for` (`PromptMismatch`, below) — so the + // admitted shape cannot exist at a resting state, only while a + // player is being asked something. Every `FrameGate::AfterChild` + // frame is still rejected, which is the shape this guard was + // written to catch: the draw's own later instruction parked + // ABOVE the draw instead of outside the pair, where + // `insert_ability_continuation_outside_active_post_replacement_draw` + // puts it (CR 608.2c: instructions run in the order written). + let paired_child_is_reachable = match self.frames.get(index + 2) { + None => true, + Some(above) => { + matches!(above.gate(), FrameGate::DirectChoice(_)) + && index + 3 == self.frames.len() + } + }; + if !paired_child_is_reachable { return Err(ResolutionStackError::InvalidAdjacentPair( - "a paired multi-draw child is not the active stack top", + "a paired multi-draw child is buried below frames other than the active direct-choice owner", )); } } @@ -3053,6 +3432,7 @@ impl ResolutionStateWire { .map_err(|error| error.to_string())?; frames.recover_draw_sequence_allocator(); frames.recover_discard_allocator(); + frames.recover_post_replacement_frame_id_allocator(); let mut state_value = value; let state_object = state_value.as_object_mut().expect("checked JSON object"); @@ -3731,6 +4111,14 @@ pub(crate) fn canonicalize_legacy_resolution_state( frames .restore_next_draw_sequence_frame_id(state.resolution_stack.next_draw_sequence_frame_id()); frames.restore_next_discard_frame_id(state.resolution_stack.next_discard_frame_id()); + // Threaded for the same reason as the two above: this function is both the + // WRITER's canonicalization (`ResolutionStateWire::to_value`) and the + // right-hand side of the v2 identity gate, and `ResolutionStack` derives + // `PartialEq`. Without this, every save in which a post-replacement dispatch + // ever occurred fails that gate on load. + frames.restore_last_post_replacement_frame_id( + state.resolution_stack.last_post_replacement_frame_id(), + ); for frame in state.resolution_stack.iter() { if !frame.is_runtime_stack_resident() { @@ -3756,6 +4144,14 @@ fn project_frames_into_legacy_state( projected .resolution_stack .restore_next_discard_frame_id(frames.next_discard_frame_id()); + // The left-hand side of the v2 identity gate. `state` is materialized from a + // value with `resolution_frames` removed and `resolution_stack` forbidden, + // so its allocator is `Default` (0); without this restore the projection's + // allocator stays 0 while `frames` carries N, and the derived `PartialEq` + // comparison rejects the payload. + projected + .resolution_stack + .restore_last_post_replacement_frame_id(frames.last_post_replacement_frame_id()); for frame in frames.iter() { match frame { ResolutionFrame::AbilityContinuation(frame) => { @@ -5133,6 +5529,99 @@ mod tests { )); } + /// **U3-a — discriminating(U3).** CR 614.11a + CR 121.6b: a replaced draw + /// inside a draw sequence completes every action the replacement requires + /// before the sequence resumes. When one of those actions is a player's + /// choice, the game rests on that choice with the draw sequence still parked + /// beneath it — so the paired multi-draw child legitimately sits one below the + /// stack top while the frame that owns the live prompt sits on it. + /// + /// The paired negative is the shipped + /// `validation_rejects_a_paused_drain_pair_buried_below_another_frame`, which + /// buries the same pair below an `AbilityContinuation` (a + /// `FrameGate::AfterChild` frame) and must stay red — that is the shape this + /// guard exists to catch: the draw's own later instruction parked ABOVE the + /// draw instead of outside the pair. + #[test] + fn validation_admits_the_live_direct_choice_owner_above_a_paused_drain_pair() { + let optional_effect_frame = || { + ResolutionFrame::OptionalEffect(OptionalEffectFrame { + ability: Box::new(resolved_draw(81)), + trigger_event: None, + trigger_events: Vec::new(), + trigger_match_count: None, + }) + }; + let opponent_may = WaitingFor::OpponentMayChoice { + player: PlayerId(1), + source_id: ObjectId(81), + description: None, + remaining: Vec::new(), + }; + + // (1) ACCEPT: the prompt owner directly above the paired child. + let mut admitted = ResolutionStack::default(); + admitted + .install_adjacent_post_replacement_draw( + paused_post_replacement_frame(), + active_multi_draw_frame(), + ) + .expect("pair installs"); + admitted + .validate(&opponent_may) + .expect("reach-guard: the bare pair is well formed before anything is stacked on it"); + admitted.push_inner(optional_effect_frame()); + admitted + .validate(&opponent_may) + .expect("the live direct-choice owner may sit above the paused pair"); + + // (2) The admitted shape is bound to the LIVE prompt: at a resting + // `Priority` the same stack is still rejected, by `PromptMismatch`. + assert!(matches!( + admitted.validate(&WaitingFor::Priority { + player: PlayerId(0) + }), + Err(ResolutionStackError::PromptMismatch { .. }) + )); + + // (3) Exactly ONE frame is admitted, and only a direct-choice one. Both + // two-frame burials stay rejected, in either order. + let mut continuation_then_prompt = ResolutionStack::default(); + continuation_then_prompt + .install_adjacent_post_replacement_draw( + paused_post_replacement_frame(), + active_multi_draw_frame(), + ) + .expect("pair installs"); + continuation_then_prompt.push_inner(continuation_frame(9)); + continuation_then_prompt.push_inner(optional_effect_frame()); + assert!(matches!( + continuation_then_prompt.validate(&opponent_may), + Err(ResolutionStackError::InvalidAdjacentPair(_)) + )); + + let mut prompt_then_continuation = ResolutionStack::default(); + prompt_then_continuation + .install_adjacent_post_replacement_draw( + paused_post_replacement_frame(), + active_multi_draw_frame(), + ) + .expect("pair installs"); + prompt_then_continuation.push_inner(optional_effect_frame()); + prompt_then_continuation.push_inner(continuation_frame(9)); + assert!(matches!( + prompt_then_continuation.validate(&opponent_may), + Err(ResolutionStackError::InvalidAdjacentPair(_)) + )); + + // (4) The admitted shape also survives the v2 wire gate, which runs + // `canonicalize` + `validate` on BOTH the write and the read side. + let mut state = GameState::new_two_player(81); + state.waiting_for = opponent_may.clone(); + serde_json::from_value::(v2_fixture_with_frames(state, admitted)) + .expect("the admitted arrangement round-trips through the v2 wire gate"); + } + #[test] fn validation_keeps_an_independent_paused_drain_without_a_draw_frame() { let mut stack = ResolutionStack::default(); @@ -5783,6 +6272,451 @@ mod tests { ); } + // ----------------------------------------------------------------------- + // Identity-addressed post-replacement dispatch rows. + // + // HANDLE-PROVENANCE RULE, and it is load-bearing: these rows live inside the + // module that defines `IdentifiedPostReplacementDispatch`, so they COULD + // write a struct literal. They must not. Every handle below comes from + // `begin_active_post_replacement_dispatch`, because a hand-built literal + // bypasses the stamp and the allocator commit and would make the row green + // for the wrong reason under exactly the mutations it exists to catch. + // ----------------------------------------------------------------------- + + fn ready_post_replacement_frame() -> PostReplacementDrainStack { + let mut drains = PostReplacementDrainStack::default(); + assert!( + drains.install( + PostReplacementDrain::ready(PostReplacementContinuation::Resolved(Box::new( + resolved_draw(81) + ))), + ResidentDrainPolicy::KeepResident, + ), + "the fixture's ready drain installs" + ); + drains + } + + fn frame_id_at(stack: &ResolutionStack, index: usize) -> Option { + match stack.iter().nth(index) { + Some(ResolutionFrame::PostReplacement(drains)) => drains.frame_id(), + other => panic!("expected a post-replacement frame at {index}, got {other:?}"), + } + } + + fn drain_count_at(stack: &ResolutionStack, index: usize) -> usize { + match stack.iter().nth(index) { + Some(ResolutionFrame::PostReplacement(drains)) => serde_json::to_value(drains) + .expect("a drain stack serializes")["drains"] + .as_array() + .map_or(0, Vec::len), + other => panic!("expected a post-replacement frame at {index}, got {other:?}"), + } + } + + /// **G1 — discriminating(U2).** CR 616.1g: a live dispatch survives a + /// parent-of-active insert that slides a frame in ABOVE its own frame. + /// + /// This is the one shipped insert that genuinely defeats the two-deep + /// positional accessor. The `PostReplacement` frame's absolute index never + /// changes — it stays 0 throughout; what changes is its distance from the + /// top, from 1 to 2. Production route: + /// `effects::append_to_pending_continuation` / `prepend_to_pending_continuation` + /// branch 3 and `effects::counters`'s counter-additions insert → + /// `GameState::insert_ability_continuation_parent_of_active` / + /// `insert_counter_additions_parent_of_active` → + /// `ResolvedFrameTransition::InsertParentOfActive` → + /// `ResolutionStack::insert_parent_of_active`. The child frames here are + /// `AbilityContinuation` helpers while branch 3's production children are its + /// own gated kinds; the accessor's geometry is frame-kind-agnostic (it + /// `matches!`es only `PostReplacement` at both probes), so the child kind is + /// immaterial and this is a stack-primitive unit row. + /// + /// Revert-failing assertion: `finish_post_replacement_dispatch(handle)` + /// returns `Some`. Resolved positionally the lookup returns `None` — the row + /// itself asserts that, at step 5 — so the call removes nothing and the entry + /// stays `Dispatching`. + #[test] + fn g1_a_parent_of_active_insert_does_not_detach_a_live_dispatch() { + let mut stack = ResolutionStack::default(); + assert_eq!( + stack.last_post_replacement_frame_id(), + 0, + "a fresh stack has allocated nothing" + ); + + stack.push_post_replacement(ready_post_replacement_frame()); + let (_, handle) = stack + .begin_active_post_replacement_dispatch() + .expect("ready drain begins dispatching"); + assert_eq!( + frame_id_at(&stack, 0), + Some(PostReplacementFrameId(1)), + "pre-increment allocation gives the first frame id 1, with no recovery pass" + ); + assert_eq!(stack.last_post_replacement_frame_id(), 1); + + stack.push_inner(continuation_frame(1)); + // Positive reach-guard: the POSITIONAL accessor still resolves here, so + // the row is not passing merely because positional addressing was + // already broken before the insert. + assert_eq!( + stack.active_post_replacement_parent_index(), + Some(0), + "reach-guard: distance from the top is 1 before the insert" + ); + + let frames_before = stack.len(); + stack + .insert_parent_of_active(continuation_frame(2)) + .expect("active child accepts an immediate parent"); + + // The three-part "distance changed while the absolute index did not" + // claim, asserted at the exact function a positional cleanup restores. + assert_eq!(stack.len(), frames_before + 1, "the insert added a frame"); + assert_eq!( + frame_id_at(&stack, 0), + Some(PostReplacementFrameId(1)), + "the PostReplacement frame's ABSOLUTE index is still 0" + ); + assert_eq!( + stack.active_post_replacement_parent_index(), + None, + "reach-guard: the frame has left the two-deep positional window" + ); + + let retired = stack.finish_post_replacement_dispatch(handle); + assert!( + retired.is_some(), + "CR 616.1g: the dispatch retires its exact entry wherever its frame now sits" + ); + assert_eq!( + drain_count_at(&stack, 0), + 0, + "the entry was removed from the ORIGINAL frame at index 0" + ); + assert_eq!( + stack.len(), + frames_before + 1, + "a buried empty frame is left in place — removing it would reorder the stack" + ); + } + + /// **G2 — discriminating(U2).** On a FRESH stack that never restored, an + /// outer dispatch never resolves to a sibling `PostReplacement` frame. + /// + /// This is the Outcome-B aliasing shape: while an outer frame is buried, + /// `GameState::install_post_replacement_drain`'s accessor misses and mints a + /// SIBLING frame above it. Burying `F1` at least two deep first is what makes + /// production take that new-frame branch at all, and it is what makes a + /// positional cleanup resolve the outer handle to `F2`. + /// + /// Revert-failing assertion: the outer finish removes `F1`'s entry while + /// `F2`'s `Ready` drain is untouched. Resolved positionally the lookup lands + /// on `F2`, whose slot is `Ready` — and `finish_dispatch` removes only a + /// `Dispatching` slot — so it returns `None`, removes nothing, and `F1`'s + /// entry survives `Dispatching`. + #[test] + fn g2_an_outer_dispatch_never_aliases_a_sibling_post_replacement_frame() { + let mut stack = ResolutionStack::default(); + assert_eq!( + stack.last_post_replacement_frame_id(), + 0, + "no recovery pass ran: this is a fresh stack" + ); + + stack.push_post_replacement(ready_post_replacement_frame()); + let (_, handle_outer) = stack + .begin_active_post_replacement_dispatch() + .expect("F1's ready drain begins dispatching"); + assert_eq!(frame_id_at(&stack, 0), Some(PostReplacementFrameId(1))); + assert_eq!(stack.last_post_replacement_frame_id(), 1); + + stack.push_inner(continuation_frame(1)); + stack.push_inner(continuation_frame(2)); + assert_eq!( + stack.active_post_replacement_parent_index(), + None, + "reach-guard: F1 is buried, which is the precondition under which \ + install_post_replacement_drain mints a sibling in production" + ); + + // The new-frame branch's non-MultiDraw arm. + stack.push_post_replacement(ready_post_replacement_frame()); + let sibling_index = stack.len() - 1; + assert_eq!( + frame_id_at(&stack, sibling_index), + None, + "a freshly minted sibling frame is unstamped and unaddressable by type" + ); + + let retired = stack.finish_post_replacement_dispatch(handle_outer); + assert!( + retired.is_some(), + "the outer dispatch retires ITS OWN entry, not the sibling's" + ); + assert_eq!(drain_count_at(&stack, 0), 0, "F1's entry is gone"); + assert_eq!( + drain_count_at(&stack, sibling_index), + 1, + "F2's Ready drain is untouched" + ); + + let (_, handle_sibling) = stack + .begin_active_post_replacement_dispatch() + .expect("F2's ready drain begins dispatching"); + assert_eq!( + handle_sibling.frame(), + PostReplacementFrameId(2), + "the sibling takes the NEXT id, distinct from F1's" + ); + } + + /// **H6(a) — legacy half.** Restored id-less `PostReplacement` frames read + /// `None`, the allocator stays 0, and the first mint yields `Some(1)` from + /// the PRE-INCREMENT — not from the recovery pass. + /// + /// Precondition: the TOP restored frame must hold a `Ready` drain, or + /// `begin_dispatch` declines and the "first mint yields 1" assertion is + /// vacuous. + #[test] + fn h6a_legacy_id_less_post_replacement_frames_restore_unstamped() { + let mut frames = ResolutionStack::default(); + frames.push_post_replacement(PostReplacementDrainStack::default()); + frames.push_post_replacement(ready_post_replacement_frame()); + + let wire = v2_fixture_with_frames(GameState::new_two_player(142), frames); + let mut restored = serde_json::from_value::(wire) + .expect("a legacy id-less v2 payload decodes") + .into_game_state(); + + assert_eq!(restored.resolution_stack.len(), 2, "both frames restored"); + assert_eq!(frame_id_at(&restored.resolution_stack, 0), None); + assert_eq!(frame_id_at(&restored.resolution_stack, 1), None); + assert_eq!( + restored.resolution_stack.last_post_replacement_frame_id(), + 0, + "no persisted id means nothing for the recovery pass to raise" + ); + + let (_, handle) = restored + .resolution_stack + .begin_active_post_replacement_dispatch() + .expect("the top restored frame holds a Ready drain"); + assert_eq!(handle.frame(), PostReplacementFrameId(1)); + assert_eq!( + frame_id_at(&restored.resolution_stack, 1), + Some(PostReplacementFrameId(1)), + "the id landed on the TOP frame specifically" + ); + assert_eq!(frame_id_at(&restored.resolution_stack, 0), None); + } + + /// **H6(b) — recovery half.** A payload whose frames carry ids 4 and 7 with + /// the outer allocator absent decodes, and the allocator is raised to 7 so + /// the next mint cannot reuse a persisted id. + /// + /// The mint-yields-8 claim cannot be asserted against the restored frames + /// themselves: both their residents are `Paused`, so `begin_dispatch` + /// declines, and an already-stamped frame would return its own id WITHOUT + /// committing the allocator. So a fresh unstamped `Ready` frame is pushed and + /// the mint runs against that. + #[test] + fn h6b_the_v2_reader_recovers_the_post_replacement_allocator() { + let mut frames = ResolutionStack::default(); + for id in [4u64, 7] { + let ResolutionFrame::PostReplacement(mut drains) = paused_post_replacement_frame() + else { + unreachable!("helper constructs a post-replacement frame") + }; + assert_eq!( + drains.stamp_frame_id(PostReplacementFrameId(id)), + PostReplacementFrameId(id) + ); + frames.push_post_replacement(drains); + } + // `stamp_frame_id` deliberately does not touch the allocator, which is + // exactly the payload this half needs — stamped frames, allocator 0. + assert_eq!(frames.last_post_replacement_frame_id(), 0); + + let wire = v2_fixture_with_frames(GameState::new_two_player(143), frames); + let mut restored = serde_json::from_value::(wire) + .expect("a stamped payload with an absent outer allocator decodes") + .into_game_state(); + assert_eq!( + restored.resolution_stack.last_post_replacement_frame_id(), + 7, + "the recovery pass raises the allocator to the maximum persisted id" + ); + assert_eq!( + frame_id_at(&restored.resolution_stack, 0), + Some(PostReplacementFrameId(4)) + ); + assert_eq!( + frame_id_at(&restored.resolution_stack, 1), + Some(PostReplacementFrameId(7)) + ); + + restored + .resolution_stack + .push_post_replacement(ready_post_replacement_frame()); + let (_, handle) = restored + .resolution_stack + .begin_active_post_replacement_dispatch() + .expect("the fresh unstamped frame holds a Ready drain"); + assert_eq!( + handle.frame(), + PostReplacementFrameId(8), + "the recovered allocator cannot reuse a persisted id" + ); + assert_eq!( + restored.resolution_stack.last_post_replacement_frame_id(), + 8 + ); + } + + /// **H6(c) — validate half.** Duplicate ids are rejected; any number of + /// unstamped frames is legal; a stamped payload whose MAXIMUM id equals + /// the allocator is accepted — that assertion is what pins the + /// comparison as `>` rather than `>=`, because this allocator holds the LAST + /// id allocated, not the next — and a stamped id ABOVE the allocator is + /// rejected, which is the branch's own negative case and the only half that + /// fails if the comparison is deleted outright rather than merely loosened. + /// + /// Rejection expectations are matched by SUBSTRING: `InvalidPayload` Displays + /// as `"invalid embedded {frame:?} frame: {message}"`. + #[test] + fn h6c_validate_rejects_duplicate_post_replacement_ids_and_admits_legal_ones() { + let mut duplicates = ResolutionStack::default(); + for _ in 0..2 { + let ResolutionFrame::PostReplacement(mut drains) = paused_post_replacement_frame() + else { + unreachable!("helper constructs a post-replacement frame") + }; + drains.stamp_frame_id(PostReplacementFrameId(4)); + duplicates.push_post_replacement(drains); + } + let wire = v2_fixture_with_frames(GameState::new_two_player(144), duplicates); + let error = serde_json::from_value::(wire) + .expect_err("a payload with duplicate post-replacement frame ids is rejected") + .to_string(); + assert!( + error.contains("duplicate post-replacement frame id"), + "unexpected rejection message: {error}" + ); + + let mut unstamped = ResolutionStack::default(); + for _ in 0..3 { + unstamped.push_post_replacement(PostReplacementDrainStack::default()); + } + let wire = v2_fixture_with_frames(GameState::new_two_player(145), unstamped); + assert!( + serde_json::from_value::(wire).is_ok(), + "any number of unstamped frames is legal — that is the carve-out" + ); + + let mut at_the_allocator = ResolutionStack::default(); + let ResolutionFrame::PostReplacement(mut drains) = paused_post_replacement_frame() else { + unreachable!("helper constructs a post-replacement frame") + }; + drains.stamp_frame_id(PostReplacementFrameId(1)); + at_the_allocator.push_post_replacement(drains); + at_the_allocator.restore_last_post_replacement_frame_id(1); + let wire = v2_fixture_with_frames(GameState::new_two_player(146), at_the_allocator); + assert!( + serde_json::from_value::(wire).is_ok(), + "a LAST-allocated allocator's valid range is 1..=last: id == allocator is legal" + ); + + // The rejection half of that same `>`: a stamped id ABOVE the allocator. + // + // Driven at `validate` DIRECTLY, and that is not a shortcut — it is the + // only place the branch is reachable. Through the v2 READ path it is + // dead: `recover_post_replacement_frame_id_allocator` runs FIRST and + // raises the allocator to the maximum persisted id, so no decoded + // payload can present this shape. `validate` is still called with no + // such recovery in front of it by the runtime invariant check + // (`debug_assert_runtime_resolution_invariants`, after a restore and + // after every public action) and by the v2 WRITE side + // (`ResolutionStateWire::to_value`), which is what this exercises. + // + // Constructed identically to `at_the_allocator` above except that the + // allocator sits one BELOW the stamped id, so a failure here can only be + // this branch and not some unrelated structural check. + let mut behind_the_allocator = ResolutionStack::default(); + let ResolutionFrame::PostReplacement(mut drains) = paused_post_replacement_frame() else { + unreachable!("helper constructs a post-replacement frame") + }; + drains.stamp_frame_id(PostReplacementFrameId(1)); + behind_the_allocator.push_post_replacement(drains); + behind_the_allocator.restore_last_post_replacement_frame_id(0); + let state = GameState::new_two_player(149); + let error = behind_the_allocator + .validate(&state.waiting_for) + .expect_err("a stamped id above the allocator is rejected") + .to_string(); + assert!( + error.contains( + "the resolution-stack post-replacement allocator is behind its active frame" + ), + "unexpected rejection message: {error}" + ); + } + + /// **H6(d) — round-trip half.** The allocator survives the real v2 write → + /// read → identity-gate round trip. + /// + /// Deliberately goes through the WRITE side (`to_value` → `canonicalize` → + /// `validate`) rather than through `v2_fixture_with_frames`, because the + /// writer is half of what the round-trip threading breaks. + #[test] + fn h6d_the_post_replacement_allocator_survives_the_v2_round_trip() { + let mut state = GameState::new_two_player(147); + state + .resolution_stack + .push_post_replacement(ready_post_replacement_frame()); + let (_, handle) = state + .resolution_stack + .begin_active_post_replacement_dispatch() + .expect("ready drain begins dispatching"); + assert!(state + .resolution_stack + .pause_post_replacement_dispatch(handle)); + + // NON-VACUITY CONTROL. Without a NONZERO allocator and a STAMPED frame, + // this row round-trips cleanly even with the threading removed — a 0/0 + // comparison passes the identity gate. It also catches the construction + // error of building the frame with a bare `drains.begin_dispatch()`, + // which does not stamp. + assert_eq!( + state.resolution_stack.last_post_replacement_frame_id(), + 1, + "non-vacuity: the allocator must be nonzero before serializing" + ); + assert_eq!( + frame_id_at(&state.resolution_stack, 0), + Some(PostReplacementFrameId(1)), + "non-vacuity: the frame must be stamped before serializing" + ); + + let v2 = serde_json::to_value(ResolutionStateWire::from_game_state(state)) + .expect("stamped v2 fixture serializes"); + let restored = serde_json::from_value::(v2) + .expect("stamped v2 payload decodes") + .into_game_state(); + + assert_eq!( + restored.resolution_stack.last_post_replacement_frame_id(), + 1, + "the allocator crossed the wire and both sides of the identity gate" + ); + assert_eq!( + frame_id_at(&restored.resolution_stack, 0), + Some(PostReplacementFrameId(1)), + "the frame kept its identity across the round trip" + ); + } + #[test] fn v1_remaining_resolution_frames_resume_via_shipped_authorities() { let mut draw_sequences = DrawSequenceStack::default(); diff --git a/crates/engine/tests/integration/fixtures/mycoloth_devour_wedge_turn15.json.gz b/crates/engine/tests/integration/fixtures/mycoloth_devour_wedge_turn15.json.gz new file mode 100644 index 0000000000..92cf98631f Binary files /dev/null and b/crates/engine/tests/integration/fixtures/mycoloth_devour_wedge_turn15.json.gz differ diff --git a/crates/engine/tests/integration/fixtures/mycoloth_devour_wedge_turn20.json.gz b/crates/engine/tests/integration/fixtures/mycoloth_devour_wedge_turn20.json.gz new file mode 100644 index 0000000000..347b7ffb64 Binary files /dev/null and b/crates/engine/tests/integration/fixtures/mycoloth_devour_wedge_turn20.json.gz differ diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 76e6d57cf7..51e1bc2f0c 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1242,6 +1242,7 @@ mod momir_token_firebreathing_duration; mod moon_girl_second_draw_base_pt; mod mox_diamond_discard_cost_2853; mod multi_source_each_power_damage; +mod mycoloth_devour_drain_strand; mod nadu_lavaspur_boots_max_times; mod najeela_extra_combat_grant_2898; mod no_top_level_test_binaries; diff --git a/crates/engine/tests/integration/mycoloth_devour_drain_strand.rs b/crates/engine/tests/integration/mycoloth_devour_drain_strand.rs new file mode 100644 index 0000000000..f4d1086b86 --- /dev/null +++ b/crates/engine/tests/integration/mycoloth_devour_drain_strand.rs @@ -0,0 +1,639 @@ +//! CR 603.3 + CR 603.3b + CR 608.2c: two wedged Mycoloth boards recover at the +//! first action boundary — turn-15 at the priority boundary that action lands +//! on, turn-20 at the entry of the boundary itself. +//! +//! Both captures show the same permanent wedge: a `PostReplacement` resolution +//! frame whose resident drain is stuck in `DrainStatus::Dispatching`. Nothing +//! can retire that entry — `begin_dispatch` refuses a `Dispatching` resident, +//! `finish_paused_dispatch` pops only a `Paused` one, and `finish_dispatch` +//! needs a transient handle that died with its dispatcher's call frame. The +//! frame therefore outlives every removal path, `resolution_stack` stays +//! non-empty forever, and `triggers::resolution_completion_can_settle` is false +//! forever. Two rules-level consequences follow, and both are visible in the +//! captures: parked triggered abilities can never be put on the stack even +//! though CR 603.3b requires them there before any player receives priority, +//! and the stale resolving carrier can never settle (CR 608.2c). +//! +//! # Fixture provenance +//! +//! Derived from the reporter's raw client dumps (Discord thread +//! 1537641754298290226). The 12 MB raw dumps are deliberately NOT tracked. +//! +//! | artifact | bytes | sha256 | +//! |---|---|---| +//! | `game-state-turn-15-2026-08-15T14-02-22-524Z.json` (raw capture) | 11 944 525 | `ec8c609c1f2ccb92d76afc536ddd10aab6e9b9d62d15f408e2e40cdb81de0107` | +//! | derived `mycoloth_devour_wedge_turn15.json.gz` | 393 743 | `b1fe83892df9ab27ce0bfd8510996b79de21ee7be10ccfcbbd30ace903359b05` | +//! | `game-state-turn-20-2026-08-15T01-13-36-601Z.json` (raw capture) | 13 351 646 | `1788737cf6d499f8878c9869546967c0aad768d8187ae2959d5cc0bc54dd6353` | +//! | derived `mycoloth_devour_wedge_turn20.json.gz` | 314 852 | `88903b1bff39c318290aa9e78fe16ffdd4769a30eec07e8a46ddaa62320f4e3a` | +//! +//! Byte-reproducible regeneration — `-n` is load-bearing, since without it gzip +//! stamps an mtime and the digest never lands: +//! +//! ```text +//! jq -c '{gameState}' .json | gzip -9 -n \ +//! > crates/engine/tests/integration/fixtures/mycoloth_devour_wedge_turn15.json.gz +//! ``` +//! +//! # What these fixtures do and do not prove +//! +//! They are post-wedge snapshots, so they prove **recovery**, not the instant of +//! stranding. `applied: []` on both captured drains is NOT provenance about how +//! the drain was installed: `apply_pending_post_replacement_effect` +//! unconditionally `std::mem::take`s the resident's `applied` set before +//! `begin_dispatch` can decline, so under the wedge that set is emptied at every +//! priority boundary regardless of its installed contents. +//! +//! They are also *legacy* payloads — no per-frame post-replacement id, no outer +//! allocator — so they exercise none of the identity-addressed dispatch wire +//! path. That is covered by the `types/resolution.rs` round-trip rows. +//! +//! This module must NOT be read as claiming the captured strand came from the +//! Devour delivery tail. What is true is narrower: the drain *shape* +//! (`source: null`, `event_source: null`) is consistent with a +//! `clear_post_replacement_source` caller, and these captures are post-wedge +//! snapshots that prove recovery rather than the instant of stranding. The one +//! producer attribution that IS measured is the **Zur's Weirding +//! draw-replacement path**, which was observed at `BASE_SHA` to leave a +//! `Dispatching` resident both mid-scenario (`[PostReplacement, MultiDraw, +//! OptionalEffect]` under `OpponentMayChoice`) and at rest (a single-frame +//! `PostReplacement` at `Priority` — the reporter's exact wedge shape). That is +//! why this file also carries `b2_zurs_weirding_replacement_leaves_no_dispatching_drain`, +//! which is NOT a Devour scenario and says so in its own doc comment. +//! +//! # What one action does — and where +//! +//! Rows **A4** and **A4b** together discriminate the *entry* evaluation point; +//! A4's red was observed at `BASE_SHA`, before the entry call site existed. +//! +//! * **turn-15**: the pass lands on `Priority` again (`priority_passes: []`), so +//! `resume_pending_continuation_if_priority`'s gate is true. The +//! priority-boundary sweeper retires the frame, +//! `settle_resolving_stack_entry_after_continuation_resume` settles the +//! carrier, and `run_post_action_pipeline`'s deferred-trigger drain runs — all +//! three within one `PassPriority`. Rows A1–A3. +//! * **turn-20**: the pass advances the phase, so that gate is **false** for the +//! resulting state and the post-action sweeper is never entered. The frame is +//! retired by the **entry** sweep in `engine::apply_action_boundary_core`, +//! which runs on the state *as found*, before `boundary_snapshot`. The parked +//! abilities then reach the stack in the same action — but by a seam this +//! change does not own (`turns::process_phase_triggers`), which is gated behind +//! `resolution_completion_can_settle` and therefore could not run at all while +//! the strand was present. A4 records the measured terminal shape and asserts +//! strand removal plus that drain; it pins nothing about the terminal +//! `waiting_for`. Row A4b proves the entry siting by repairing the state on an +//! action the engine rejects. + +use engine::game::engine::apply; +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::actions::{DebugAction, GameAction}; +use engine::types::counter::CounterType; +use engine::types::game_state::{GameState, PersistedGameState, WaitingFor}; +use engine::types::identifiers::ObjectId; +use engine::types::phase::Phase; +use engine::types::player::PlayerId; +use engine::types::zones::Zone; + +fn gunzip(gz: &[u8]) -> String { + use std::io::Read; + let mut json = String::new(); + flate2::read::GzDecoder::new(gz) + .read_to_string(&mut json) + .expect("fixture .json.gz must inflate to UTF-8 JSON"); + json +} + +/// Load a capture's `["gameState"]` through the REAL production restore +/// chokepoint `PersistedGameState::into_game_state` — never a bare `GameState` +/// decode, which would skip `reject_legacy_raw_prompt_authority` and +/// `decode_persisted_resolution_state`. +/// +/// One projection is required first, and it is worth stating exactly why rather +/// than hiding it in a helper. `client/src/services/gameStateExport.ts` writes a +/// **debug snapshot of the runtime `GameState`**, not a persistence-wire save: +/// it carries the raw `resolution_stack` field and no `resolution_state_version`. +/// `PersistedGameState`'s decoder stamps an absent version as v1, and the v1 +/// reader rejects any payload carrying `resolution_stack` outright — that +/// rejection is correct, because v1 predates typed frames entirely. +/// +/// So the snapshot is first projected onto the v2 wire, which is precisely the +/// transformation `ResolutionStateWire::to_value` performs when persisting a +/// live state: move `resolution_stack` to `resolution_frames` and stamp version +/// 2. Nothing else is touched — in particular the wedged frame and its +/// `Dispatching` drain cross verbatim. The decode then runs the FULL v2 reader: +/// the three allocator recovery passes, `ResolutionStack::validate`, +/// `project_frames_into_legacy_state` → `canonicalize_legacy_resolution_state` +/// and the derived-`PartialEq` identity gate, and +/// `validate_trigger_firing_coherence` — a strictly stronger chokepoint than the +/// v1 path, not a weaker one. +fn load_capture(gz: &[u8]) -> GameState { + let json = gunzip(gz); + let envelope: serde_json::Value = + serde_json::from_str(&json).expect("dump envelope parses as JSON"); + let mut snapshot = envelope["gameState"].clone(); + { + let object = snapshot + .as_object_mut() + .expect("a captured gameState is a JSON object"); + assert!( + !object.contains_key("resolution_state_version"), + "the reporter's capture is an unversioned runtime debug snapshot" + ); + let stack = object + .remove("resolution_stack") + .expect("the wedged capture carries a runtime resolution_stack"); + object.insert("resolution_frames".to_string(), stack); + object.insert( + "resolution_state_version".to_string(), + serde_json::Value::from(2), + ); + } + serde_json::from_value::(snapshot) + .expect("the projected snapshot deserializes through the production decoder") + .into_game_state() +} + +fn load_turn15() -> GameState { + load_capture(include_bytes!( + "fixtures/mycoloth_devour_wedge_turn15.json.gz" + )) +} + +fn load_turn20() -> GameState { + load_capture(include_bytes!( + "fixtures/mycoloth_devour_wedge_turn20.json.gz" + )) +} + +/// The per-`PostReplacement`-frame drain statuses of the LOADED runtime state, +/// read back through the stack's own `Serialize` impl. `PostReplacementDrainStack` +/// exposes only its resident, so this is how a test observes a multi-entry +/// strand without widening production API for a test's convenience. +fn post_replacement_drain_statuses(state: &GameState) -> Vec> { + let value = + serde_json::to_value(&state.resolution_stack).expect("the resolution stack serializes"); + value["frames"] + .as_array() + .expect("frames is an array") + .iter() + .filter(|frame| frame["type"] == "PostReplacement") + .map(|frame| { + frame["data"]["drains"] + .as_array() + .expect("a post-replacement frame carries a drains array") + .iter() + .map(|drain| match &drain["status"] { + serde_json::Value::String(status) => status.clone(), + // `DrainStatus::Ready(_)` is externally tagged. + serde_json::Value::Object(map) => { + map.keys().next().cloned().unwrap_or_default() + } + other => other.to_string(), + }) + .collect() + }) + .collect() +} + +fn deferred_sources(state: &GameState) -> Vec { + state + .deferred_triggers + .iter() + .map(|deferred| deferred.pending.source_id.0) + .collect() +} + +fn deferred_descriptions(state: &GameState) -> Vec { + state + .deferred_triggers + .iter() + .map(|deferred| deferred.pending.description.clone().unwrap_or_default()) + .collect() +} + +/// Every trigger source id that reached a CR 603.3b destination: on the stack, +/// or inside an in-flight APNAP ordering pass (three same-controller triggers +/// legitimately raise an ordering prompt before they are put on the stack). +fn triggers_reaching_the_stack(state: &GameState) -> Vec { + let mut sources: Vec = state.stack.iter().map(|entry| entry.source_id.0).collect(); + if let Some(order) = &state.pending_trigger_order { + for group in &order.groups { + sources.extend(group.triggers.iter().map(|t| t.pending.source_id.0)); + } + } + sources.sort_unstable(); + sources +} + +fn p1p1(state: &GameState, id: ObjectId) -> u32 { + state + .objects + .get(&id) + .expect("object present") + .counters + .get(&CounterType::Plus1Plus1) + .copied() + .unwrap_or(0) +} + +/// Reach-guard for the turn-15 board: the wedge must be genuinely present in the +/// loaded state, or every row below is vacuous. If `into_game_state()` ever +/// normalises the wedged `resolution_stack` away, this reds immediately rather +/// than letting the recovery rows pass for the wrong reason. +fn assert_turn15_wedge_present(state: &GameState) { + assert!( + matches!(state.waiting_for, WaitingFor::Priority { player } if player == PlayerId(1)), + "the capture is parked on P1 priority, got {:?}", + state.waiting_for + ); + assert_eq!( + post_replacement_drain_statuses(state), + vec![vec!["Dispatching".to_string()]], + "exactly one PostReplacement frame carrying exactly one Dispatching drain" + ); + assert_eq!( + state.resolution_stack.len(), + 1, + "the wedge is a single-frame resolution stack" + ); + assert_eq!( + deferred_sources(state), + vec![52, 52, 199], + "the three parked triggers are two source-52 dies drains and one source-199 sacrifice draw" + ); + let descriptions = deferred_descriptions(state); + assert_eq!( + descriptions[0], descriptions[1], + "the two source-52 firings share one ability" + ); + assert!( + descriptions[0] + .contains("Whenever a creature you control dies, each opponent loses 1 life"), + "source 52 is the Bastion-of-Remembrance dies drain, got {:?}", + descriptions[0] + ); + assert!( + descriptions[2].contains("Whenever you sacrifice a creature, draw a card"), + "source 199 is the sacrifice-draw ability, got {:?}", + descriptions[2] + ); + let carrier = state + .resolving_stack_entry + .as_ref() + .expect("the capture carries a stale resolving stack entry"); + assert_eq!(carrier.id, ObjectId(97), "the stale carrier is object 97"); + assert_eq!( + state.objects[&ObjectId(97)].zone, + Zone::Graveyard, + "object 97 (Witherbloom Command) has already finished resolving" + ); + assert_eq!( + state.objects[&ObjectId(84)].zone, + Zone::Battlefield, + "Mycoloth is on the battlefield" + ); + assert_eq!( + p1p1(state, ObjectId(84)), + 4, + "Devour 2 x 2 creatures landed 4 +1/+1 counters — this is not a counter defect" + ); +} + +/// Reach-guard for the turn-20 board: the multi-entry strand shape. +fn assert_turn20_wedge_present(state: &GameState) { + assert!( + matches!(state.waiting_for, WaitingFor::Priority { player } if player == PlayerId(1)), + "the capture is parked on P1 priority, got {:?}", + state.waiting_for + ); + assert_eq!( + post_replacement_drain_statuses(state), + vec![vec!["Dispatching".to_string(), "Dispatching".to_string()]], + "one PostReplacement frame carrying TWO Dispatching drains — the multi-entry strand" + ); + assert_eq!( + deferred_sources(state), + vec![157, 199], + "two parked triggers: source 157 and the source-199 sacrifice draw" + ); + assert!( + state.resolving_stack_entry.is_some(), + "the capture carries a stale resolving stack entry" + ); +} + +/// **A1 — discriminating(U1).** CR 603.3b + CR 608.2c: the orphaned +/// `PostReplacement` frame is retired at the first priority boundary, so the +/// resolution stack empties. +/// +/// Revert-failing assertion: `state.resolution_stack.is_empty()`. Without the +/// ownerless-strand sweep the dispatcher declines the `Dispatching` resident, +/// the `is_empty` frame-removal block is gated false because the strand is still +/// resident, and the frame survives — the assertion reads 1. +#[test] +fn a1_wedged_turn15_capture_retires_its_ownerless_dispatching_frame() { + let mut state = load_turn15(); + assert_turn15_wedge_present(&state); + + apply(&mut state, PlayerId(1), GameAction::PassPriority).expect("PassPriority is legal"); + + assert!( + state.resolution_stack.is_empty(), + "the ownerless Dispatching drain and its now-empty frame must both be gone, got {:?}", + post_replacement_drain_statuses(&state) + ); +} + +/// **A2 — discriminating(U1).** CR 603.3b: with the wedge cleared, the three +/// parked triggered abilities are put on the stack before any player receives +/// priority. +/// +/// Revert-failing assertion: `deferred_triggers.is_empty()` plus the three +/// source ids reaching a CR 603.3b destination. Under the wedge +/// `resolution_completion_can_settle` is false forever, so +/// `can_drain_deferred_triggers` is false forever and all three stay parked. +#[test] +fn a2_wedged_turn15_capture_drains_its_parked_triggers() { + let mut state = load_turn15(); + assert_turn15_wedge_present(&state); + + apply(&mut state, PlayerId(1), GameAction::PassPriority).expect("PassPriority is legal"); + + assert!( + state.deferred_triggers.is_empty(), + "all three parked abilities must leave the deferred queue, still parked: {:?}", + deferred_sources(&state) + ); + assert_eq!( + triggers_reaching_the_stack(&state), + vec![52, 52, 199], + "both source-52 dies-drain firings and the source-199 sacrifice draw reach the stack" + ); +} + +/// **A3 — discriminating(U1).** CR 608.2c: the stale resolving carrier settles +/// once the resolution stack can complete. +/// +/// Revert-failing assertion: `resolving_stack_entry.is_none()`. Under the wedge +/// `resolving_stack_entry_can_settle` is false forever and the carrier stays +/// `Some(97)` — a Witherbloom Command already in the graveyard, read as live by +/// `trigger_matchers`, `zones` and `bounce` on every later resolution. +#[test] +fn a3_wedged_turn15_capture_settles_its_stale_resolving_carrier() { + let mut state = load_turn15(); + assert_turn15_wedge_present(&state); + + apply(&mut state, PlayerId(1), GameAction::PassPriority).expect("PassPriority is legal"); + + assert!( + state.resolving_stack_entry.is_none(), + "the stale carrier must settle, got {:?}", + state.resolving_stack_entry.as_ref().map(|entry| entry.id) + ); +} + +/// **A4 — discriminating(U1), of the ENTRY evaluation point.** A wedge that is +/// *sitting at* a rest boundary when the state is loaded recovers even though +/// this action does not land on `Priority`; and the multi-entry strand recovers +/// in one boundary, because the sweep loops until it meets a `Ready` or `Paused` +/// resident, so BOTH stranded drains retire and the emptied frame is removed. +/// +/// Revert-failing assertion: `resolution_stack.is_empty()` on a board whose one +/// frame carried two `Dispatching` entries. The discriminating patch is **(a2)** +/// — reverting only the two call lines at `engine::apply_action_boundary_core`'s +/// entry, leaving the priority-boundary sweep intact. **A4b** is the row that +/// isolates that entry point with no unmeasured premise, by repairing a state on +/// an action the engine REJECTS. +/// +/// CORRECTION, recorded because an earlier round pinned assertions on it: the +/// `"Priority -> DeclareAttackers"` framing was a **MID-ACTION** reading taken at +/// the sweep hook inside `resume_pending_continuation_if_priority`, not the state +/// `apply` returns. `run_auto_pass_loop`'s `DeclareAttackers` arm auto-submits an +/// empty attack set on exactly this fixture's shape (`valid_attacker_ids` empty, +/// `phase_stops` absent by serde default), so the action does not end there +/// either. The settled post-action state is MEASURED rather than read: +/// +/// ```text +/// waiting_for = Priority { player: PlayerId(1) } phase = EndCombat +/// drains = [] deferred = [] stack.len() = 2 +/// resolving_stack_entry = Some(ObjectId(430)) pending_completion = false +/// ``` +/// +/// **CR 603.3 + CR 603.3b:** both parked abilities reach the stack — the queue +/// empties and `stack.len()` goes from 0 to 2 — and the board rests at `Priority` +/// with no resolution frame. That is a TWO-DEFECT STACK resolving, and the second +/// half is not this change's work: `turns::process_phase_triggers` drains the +/// parked queue at a phase boundary. That drain is gated behind +/// `triggers::can_drain_deferred_triggers`, whose first condition is +/// `!resolution_completion_can_settle(state)`, and an ownerless `Dispatching` +/// strand pins that predicate false forever. So this fix is the PRECONDITION for +/// that one: without the strand removal the queue could not drain no matter how +/// many boundaries offered it the chance. +/// +/// What this row therefore claims is still strand removal, plus the drain the +/// strand was blocking — including the ARRIVAL half of that drain, since an +/// emptied queue alone is equally satisfied by a discarded one. It pins NOTHING +/// about the terminal `waiting_for` — that shape is recorded above, not +/// asserted, because it is produced by a seam this change does not own. +#[test] +fn a4_wedged_turn20_capture_retires_both_stranded_drains() { + let mut state = load_turn20(); + assert_turn20_wedge_present(&state); + + apply(&mut state, PlayerId(1), GameAction::PassPriority).expect("PassPriority is legal"); + + assert!( + state.resolution_stack.is_empty(), + "both stranded drains and their frame must be gone, got {:?}", + post_replacement_drain_statuses(&state) + ); + assert!( + state.deferred_triggers.is_empty(), + "CR 603.3 + CR 603.3b: with the strand gone, `resolution_completion_can_settle` is true \ + again and the parked abilities must LEAVE the deferred queue, still parked: {:?}", + deferred_sources(&state) + ); + // The paired positive, which A2 already carries for turn-15. Emptiness alone + // is satisfied by a queue that was DISCARDED as well as by one that drained, + // and discarding is a CR 603.3b violation that this row would otherwise pass. + // Non-vacuous by construction: `assert_turn20_wedge_present` pins the input + // queue to exactly these two sources before the action runs. + assert_eq!( + triggers_reaching_the_stack(&state), + vec![157, 199], + "CR 603.3b: both parked abilities must ARRIVE on the stack, not merely leave the queue" + ); +} + +/// **A4b — discriminating(U1), the ENTRY evaluation point's isolator.** +/// +/// The wedge is repaired even on an action the engine REJECTS. This is the only +/// row in the file whose green cannot be produced by the post-action +/// priority-boundary sweeper: an action that fails `check_actor_authorization` +/// never reaches `apply_action`, so `pass_priority_once_with_pipeline`, +/// `resume_pending_continuation_if_priority` and `run_post_action_pipeline` are +/// never called at all. What is left is the entry sweep in +/// `apply_action_boundary_core`, and the fact that it runs BEFORE +/// `let boundary_snapshot = state.clone();` — the snapshot every failure path +/// restores. A sweep sited after the snapshot would be rolled back with the +/// rejected action and this row would red. +/// +/// CR 603.3 + CR 603.3b + CR 608.2c: an ownerless `Dispatching` resident is a +/// corrupt state, not a rules state, so removing it is not part of any action +/// and must survive an action's rollback. +#[test] +fn a4b_entry_sweep_repairs_the_wedge_even_on_a_rejected_action() { + let mut state = load_turn20(); + assert_turn20_wedge_present(&state); + + // Player 0 does not hold priority (the capture rests at `Priority { player: 1 }`), + // so `check_actor_authorization` rejects this before any reducer arm runs. + // The `is_err` assertion is this row's reach-guard: if the action were ever + // accepted, the row would fail here rather than silently measure the + // accepted path. + let rejected = apply(&mut state, PlayerId(0), GameAction::PassPriority); + assert!( + rejected.is_err(), + "reach-guard: this action must be REJECTED, or the row measures the accepted path instead" + ); + + assert!( + state.resolution_stack.is_empty(), + "the entry sweep must repair the state even though the action was rejected, got {:?}", + post_replacement_drain_statuses(&state) + ); +} + +/// Verbatim from the shipped constant of the same name in +/// `crates/engine/tests/integration/issue_5657_zurs_weirding.rs` — inherited, not +/// paraphrased, so a later `/card-test` audit reads it as the co-witness rows' +/// own Oracle text. +const ZURS_WEIRDING_ORACLE: &str = "If a player would draw a card, they reveal it instead. Then any other player may pay 2 life. If a player does, put that card into its owner's graveyard. Otherwise, that player draws a card."; + +/// **B2' — discriminating(U2 at the mid-scenario sample, U1 at the terminal +/// sample).** A real production replacement-continuation producer leaves no +/// `Dispatching` drain anywhere in the resolution stack at ANY point in its +/// scenario. +/// +/// This is NOT a Devour scenario. It drives the Zur's Weirding draw-replacement +/// path — `replacement.rs`'s draw replacement → `OpponentMayChoice` fan-out → +/// the same single dispatcher this change fixes — because that is the producer +/// whose strand was MEASURED, rather than hypothesised, at `BASE_SHA`. +/// +/// Its red was established by that measurement, not by running this row at +/// `BASE_SHA`: the recorded capture shows `resident=Dispatching` at TWO sample +/// points in all three shipped `issue_5657_zurs_weirding` rows — mid-scenario +/// (`len=3`, `waiting_for="OpponentMayChoice"`, frames +/// `[PostReplacement, MultiDraw, OptionalEffect]`, which is U2's witness because +/// the frame is buried where the two-deep positional accessor cannot see it) and +/// at rest (`len=1`, `waiting_for="Priority"`, the reporter's exact wedge shape, +/// which is U1's). Those three shipped rows are this row's co-witnesses, and they +/// carried this exact strand invisibly for their whole history, because the +/// invariant that would have caught it keys on `Paused`. +#[test] +fn b2_zurs_weirding_replacement_leaves_no_dispatching_drain() { + // CR 603.3 + CR 603.3b + CR 608.2c: at NO point in this scenario may any + // post-replacement frame anywhere in the resolution stack hold a + // `Dispatching` entry. Sampling only at the end would let a strand that is + // created and cleaned up mid-scenario pass unseen. + // + // A whole-stack walk is required rather than `active_post_replacement_drains()`, + // because the strand this row targets was MEASURED buried at BASE_SHA + // (frame index 0 of 3, `[PostReplacement, MultiDraw, OptionalEffect]`, + // `waiting_for = OpponentMayChoice`) — precisely the shape the two-deep + // positional accessor cannot see and U1's sweep therefore cannot reach. + let assert_no_dispatching = |state: &GameState, at: &str| { + let statuses = post_replacement_drain_statuses(state); + assert!( + !statuses + .iter() + .any(|frame| frame.iter().any(|s| s == "Dispatching")), + "a Dispatching post-replacement drain survives at {at}: {statuses:?}" + ); + }; + + // The shipped block-scoped builder shape: the handle borrows `scenario` + // mutably, so it is bound inside a block rather than chained. + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + { + let mut zurs_weirding = + scenario.add_creature_from_oracle(P0, "Zur's Weirding", 0, 1, ZURS_WEIRDING_ORACLE); + zurs_weirding.as_enchantment(); + } + scenario.with_library_top(P1, &["Grizzly Bears", "Forest", "Plains"]); + scenario.with_library_top(P0, &["P0 Library 1", "P0 Library 2", "P0 Library 3"]); + let mut runner = scenario.build(); + runner.state_mut().debug_mode = true; + + let p0_life_before = runner.state().players[P0.0 as usize].life; + assert_no_dispatching(runner.state(), "before the draw"); + + runner + .act(GameAction::Debug(DebugAction::DrawCards { + player_id: P1, + count: 1, + })) + .expect("debug draw must succeed"); + assert_no_dispatching(runner.state(), "after the debug draw"); + + // Take the ACCEPT path: it runs the full replacement tail and puts the card + // into its owner's graveyard, so the positive reach-guards below are real. + let mut answered = 0; + for _ in 0..120 { + match runner.state().waiting_for.clone() { + WaitingFor::OpponentMayChoice { player, .. } => { + assert_ne!( + player, P1, + "the drawing player must never be offered the opponent-may choice" + ); + answered += 1; + runner + .act(GameAction::DecideOptionalEffect { accept: true }) + .expect("opponent-may decision must succeed"); + assert_no_dispatching(runner.state(), "after the OpponentMayChoice answer"); + } + WaitingFor::Priority { .. } if runner.state().stack.is_empty() && answered > 0 => break, + _ => { + if runner.act(GameAction::PassPriority).is_err() { + runner.advance_until_stack_empty(); + assert_no_dispatching(runner.state(), "after advance_until_stack_empty"); + break; + } + assert_no_dispatching(runner.state(), "after a priority pass"); + } + } + } + runner.advance_until_stack_empty(); + assert_no_dispatching(runner.state(), "after the scenario settles"); + + // Positive reach-guards, asserted before the negative is trusted: the + // replacement demonstrably RAN. Without these the whole-stack walk could be + // green simply because the replacement path was never entered. + assert_eq!( + answered, 1, + "reach-guard: the OpponentMayChoice fan-out must have been offered exactly once" + ); + let p1_graveyard: Vec = runner.state().players[P1.0 as usize] + .graveyard + .iter() + .filter_map(|id| runner.state().objects.get(id).map(|o| o.name.clone())) + .collect(); + let p1_hand: Vec = runner.state().players[P1.0 as usize] + .hand + .iter() + .filter_map(|id| runner.state().objects.get(id).map(|o| o.name.clone())) + .collect(); + assert!( + p1_graveyard.contains(&"Grizzly Bears".to_string()), + "reach-guard: accepting must bin the revealed card in its owner's graveyard, got {p1_graveyard:?}" + ); + assert!( + !p1_hand.contains(&"Grizzly Bears".to_string()), + "reach-guard: the binned card must not reach the drawing player's hand, got {p1_hand:?}" + ); + assert_eq!( + runner.state().players[P0.0 as usize].life, + p0_life_before - 2, + "reach-guard: the accepting player must have paid exactly 2 life" + ); +} diff --git a/scripts/check-resolution-frame-boundaries.sh b/scripts/check-resolution-frame-boundaries.sh index 3d568a60db..0bc20dc2e8 100755 --- a/scripts/check-resolution-frame-boundaries.sh +++ b/scripts/check-resolution-frame-boundaries.sh @@ -5,9 +5,27 @@ # ResolutionStateWire v1 reader, its legacy wire structures/inventory, or test # fixtures. Runtime resolution work is represented by typed ResolutionFrame # payloads; identically named typed payload members are not wire keys. The -# frame stack also permits only top access or a captured adjacent-pair boundary: -# searching the vector for a frame or removing an arbitrary index breaks that -# authority. +# frame stack permits top access, a captured adjacent-pair boundary, or +# identity-addressed access through its SINGLE named accessor. Removing an +# arbitrary index, or searching the vector anywhere else, breaks that authority. +# +# On the identity-addressed exemption: the rule is "one search, in +# `post_replacement_frame_index`", not "any search that looks identity-shaped". +# Anchoring to the function name rather than to a closure pattern is what keeps +# this from becoming a loophole — a second call site cannot acquire its own +# search by mimicking the expression, and moving or renaming the accessor fails +# the guard loudly instead of silently widening it. Each half of that sentence +# is CHECKED below rather than left to the reader: the accessor must be defined +# exactly once, must contain exactly one search, and that search must select on +# `frame_id() == Some(id)`. A span-only exemption would enforce the weaker +# "searches only there" while this comment claimed the stronger "one search, +# there" — the gap being closed here. The distinction being drawn +# is between positional/adjacency-inferred access, which GUESSES a structural +# relationship the stack does not guarantee, and identity-addressed access, +# which asserts one: ids come from a monotonic allocator that never rewinds, so +# a stale id matches nothing rather than aliasing a later frame. This mirrors +# `DrawSequenceStack::frame_mut` / `active_if` / `pop`, which is the same access +# mode on a sibling frame stack. set -euo pipefail @@ -337,7 +355,6 @@ for file_name in files: if path != resolution_path: continue - production_spans = test_spans remove_pattern = re.compile( r"\b(?:self\s*\.\s*)?frames\s*\.\s*" r"(?:remove|swap_remove|retain|drain|truncate|clear)\s*\(" @@ -348,12 +365,62 @@ for file_name in files: r"\s*\.\s*(?:position|rposition|find|find_map|any|next|nth)\s*\(", re.DOTALL, ) - for pattern, message in [ - (remove_pattern, "arbitrary ResolutionStack frame removal is forbidden; use a checked top-only API"), - (search_pattern, "generic ResolutionStack frame search is forbidden; use top or adjacent-pair access"), + + # `frames` may be searched in EXACTLY ONE production site: the module's + # single identity-addressed accessor. A span-only exemption would enforce + # "searches only there" while this script's header promises "one search, + # there" -- so the three properties that make the header true are checked + # rather than assumed: + # + # 1. exactly one `fn post_replacement_frame_index` exists. `function_span` + # takes the first textual match, so a second definition would silently + # decide which one is exempt; + # 2. its span holds exactly one search, so the exemption cannot be widened + # from the inside by adding a second search beside the first; + # 3. that search matches on `frame_id() == Some(id)`, so what is exempted + # is an identity lookup and not a positional or first-match probe + # wearing the accessor's name. + # + # The removal patterns are NOT exempted here: the accessor reads `frames` + # and never restructures it. `function_span` raises when its target is + # missing, so deleting or renaming the accessor fails the guard rather than + # quietly removing the anchor. + accessor = "post_replacement_frame_index" + definitions = len(re.findall(rf"\bfn\s+{re.escape(accessor)}\s*\(", resolution_source)) + if definitions != 1: + failures.append( + f" {resolution_path}: expected exactly one `fn {accessor}` " + f"definition, found {definitions}" + ) + accessor_start, accessor_end = function_span(resolution_source, accessor) + accessor_body = resolution_source[accessor_start:accessor_end] + searches = len(search_pattern.findall(accessor_body)) + if searches != 1: + failures.append( + f" {resolution_path}: `{accessor}` is the single sanctioned " + f"`frames` search; found {searches}" + ) + if not re.search(r"\bframe_id\s*\(\s*\)\s*==\s*Some\s*\(\s*id\s*\)", accessor_body): + failures.append( + f" {resolution_path}: `{accessor}` must select frames by identity " + "(`frame_id() == Some(id)`), not by position" + ) + + for pattern, message, spans in [ + ( + remove_pattern, + "arbitrary ResolutionStack frame removal is forbidden; use a checked top-only API", + test_spans, + ), + ( + search_pattern, + "generic ResolutionStack frame search is forbidden; use top access, " + "adjacent-pair access, or the single identity accessor", + test_spans + [(accessor_start, accessor_end)], + ), ]: for match in pattern.finditer(source): - if not in_any_span(match.start(), production_spans): + if not in_any_span(match.start(), spans): fail(failures, path, source, match.start(), message) if failures: