fix(engine): give the Cipher encode offer a frame instead of clobbering the live prompt (#7470)

crates/engine/src/game/cipher.rs

@@ -127,11 +127,18 @@ pub(crate) fn finish_encode(
 }
 
 /// CR 702.99a: Begin the on-resolution encode offer for a Cipher spell. Returns
-/// `true` when resolution paused for the choice (the caller must stop finalizing
-/// the spell and return, leaving the card held off the stack like a mutating
-/// spell), or `false` when there is no encode to offer — the spell isn't an
-/// encodable cipher card, or the controller has no creature to host it — so the
-/// caller routes the card normally (to its owner's graveyard).
+/// `true` when this hook has taken the card off the caller's hands — normally
+/// because resolution paused for the choice (the caller stops finalizing the
+/// spell and returns, leaving the card held off the stack like a mutating
+/// spell), and in the one degenerate case below because the offer already
+/// completed as a decline and routed the card itself. Returns `false` when there
+/// is no encode to offer at all — the spell isn't an encodable cipher card, or
+/// the controller has no creature to host it — so the caller routes the card
+/// normally (to its owner's graveyard).
+///
+/// Both `true` arms leave the caller with nothing to route, which is what makes
+/// them one answer: the distinction that matters to a caller is whether the card
+/// is still its responsibility.
 pub fn begin_encode_choice(state: &mut GameState, card_id: ObjectId, controller: PlayerId) -> bool {
     if !spell_can_encode(state, card_id) {
         return false;
@@ -140,14 +147,125 @@ pub fn begin_encode_choice(state: &mut GameState, card_id: ObjectId, controller:
     if creatures.is_empty() {
         return false;
     }
-    state.waiting_for = WaitingFor::CipherEncodeChoice {
-        player: controller,
+    let pending = crate::types::resolution::PendingCipherEncode {
+        stage: crate::types::resolution::CipherEncodeStage::Parked,
         card_id,
+        controller,
         creatures,
     };
+
+    // CR 702.99a: the encode is the spell's LAST instruction. When the spell's
+    // own effects are still paused on a player answer, this offer must not
+    // overwrite that live prompt (issue #7470) — it is parked BELOW the frame
+    // that owns the prompt and armed by `resume_resolution_frames` once that
+    // owner is consumed. Either way the caller's contract is the same: the
+    // resolution owes an answer, so the card is held off the stack.
+    park_encode_offer(state, pending);
     true
 }
 
+/// Park the encode offer, arming it immediately only when nothing else owns the
+/// current prompt.
+///
+/// The offer always leaves this function accounted for: armed as the live
+/// prompt, parked as a frame that will arm later, or — if the stack refuses a
+/// prompt-less frame at all — completed as a decline. It is never dropped.
+fn park_encode_offer(
+    state: &mut GameState,
+    pending: crate::types::resolution::PendingCipherEncode,
+) {
+    // The question is not what SHAPE the top frame has — it is whether the
+    // resolution is currently asking the player anything at all. Keying this to
+    // `FrameGate::DirectChoice` missed the discard pause (Mental Vapors), whose
+    // frame owns a prompt without being a direct-choice owner. `waiting_for`
+    // is the engine's single answer to "is a question open", so ask it.
+    let resolution_paused = !matches!(state.waiting_for, WaitingFor::Priority { .. });
+    if !resolution_paused {
+        let (player, card_id, creatures) = (
+            pending.controller,
+            pending.card_id,
+            pending.creatures.clone(),
+        );
+        // The frame and the prompt it may consume are installed as one step:
+        // a direct-choice owner that is visible with an unrelated `WaitingFor`
+        // is the very state #7470 left behind, and this authority makes the two
+        // unable to disagree.
+        let armed = crate::types::resolution::ResolutionFrame::CipherEncode(
+            crate::types::resolution::PendingCipherEncode {
+                stage: crate::types::resolution::CipherEncodeStage::Armed,
+                ..pending
+            },
+        );
+        if state
+            .install_direct_choice_frame(
+                armed,
+                WaitingFor::CipherEncodeChoice {
+                    player,
+                    card_id,
+                    creatures,
+                },
+            )
+            .is_err()
+        {
+            // Same reasoning as the parked branch below: a refusal means the
+            // stack was already invalid, and the card still has to leave
+            // resolution by a legal route, so the offer completes as a decline
+            // (CR 608.2n) rather than being dropped.
+            handle_encode_choice(state, card_id, None, &mut Vec::new());
+        }
+        return;
+    }
+    // Where a prompt-less frame may sit is a property of the stack's shape, not
+    // a guess this caller gets to make: an empty stack (a discard prompt owns no
+    // frame), the ordinary position below the active child, or outside a paused
+    // post-replacement/draw pair whose adjacency `validate` protects. The stack
+    // answers that itself, so no legal shape can refuse the offer.
+    let card_id = pending.card_id;
+    if state
+        .park_cipher_encode_beneath_live_prompt(pending)
+        .is_err()
+    {
+        // The stack rejected a frame that owns no prompt, which means it was
+        // already invalid before this offer existed. The card must still leave
+        // resolution by one of its two legal routes, so complete the offer the
+        // way a declined one completes (CR 608.2n: the card goes to its owner's
+        // graveyard) instead of dropping it and stranding the card off the
+        // stack. The live prompt is untouched either way — a decline moves a
+        // card, it does not ask a question.
+        handle_encode_choice(state, card_id, None, &mut Vec::new());
+    }
+}
+
+/// CR 702.99a: Arm a parked encode offer once it reaches the stack top, i.e.
+/// after the spell's own effects have finished. Called from the exhaustive
+/// frame-resume dispatch, which is what guarantees a parked offer is never
+/// forgotten.
+pub(crate) fn arm_parked_encode_offer(state: &mut GameState) {
+    let Some(pending) = state.resolution_stack.active_cipher_encode() else {
+        return;
+    };
+    // CR 702.99a: re-read legal hosts — the spell's own effects ran since the
+    // offer was parked and may have changed the board.
+    let creatures = legal_encode_creatures(state, pending.controller);
+    let (player, card_id) = (pending.controller, pending.card_id);
+    if creatures.is_empty() {
+        // No legal host left: consume the frame and route the card the way a
+        // declined offer does (CR 608.2n).
+        let _ = state.take_active_cipher_encode_frame();
+        handle_encode_choice(state, card_id, None, &mut Vec::new());
+        return;
+    }
+    if let Some(frame) = state.resolution_stack.active_cipher_encode_mut() {
+        frame.stage = crate::types::resolution::CipherEncodeStage::Armed;
+        frame.creatures = creatures.clone();
+    }
+    state.waiting_for = WaitingFor::CipherEncodeChoice {
+        player,
+        card_id,
+        creatures,
+    };
+}
+
 /// CR 702.99a–b: Resolve the encode choice. `creature = Some(id)` encodes the
 /// card on that creature (exile + link); `None` — or a creature that is no
 /// longer a legal host — declines, routing the card to its owner's graveyard

crates/engine/src/game/effects/mod.rs

@@ -988,6 +988,12 @@ pub(crate) fn resume_resolution_frames(state: &mut GameState, events: &mut Vec<G
             drain_pending_continuation(state, events)
         }
         ResolutionFrame::Discard(_) => {}
+        // CR 702.99a: a Cipher encode offer parked under the spell's own prompt
+        // arms here — i.e. only once that prompt's owner is consumed, which is
+        // what puts the encode after the spell's other effects (issue #7470).
+        ResolutionFrame::CipherEncode(_) => {
+            crate::game::cipher::arm_parked_encode_offer(state);
+        }
         ResolutionFrame::RepeatFor(_) => drain_active_repeat_for(state, events),
         ResolutionFrame::RepeatUntil(_) => drain_active_repeat_until(state),
         ResolutionFrame::RepeatedOptionalPayment(_) => {

crates/engine/src/game/engine.rs

@@ -19033,9 +19033,37 @@ mod stage2_injector_tests {
                 // three digests this log has carried since the first merge.
                 // `PreviousEffectCount` classification adds one line above all three producers,
                 // so they move uniformly to `:7003/:7080/:10318`; no prompt site changes.
-                "game/effects/mod.rs:7003".to_string(),
-                "game/effects/mod.rs:7080".to_string(),
-                "game/effects/mod.rs:10318".to_string(),
+                //
+                // MERGE OF `origin/main` INTO THE #7470 BRANCH (Cipher encode offer
+                // parked as a resolution frame): `:7003/:7080/:10318 ⇒
+                // `:7009/:7086/:10324`, a UNIFORM `+6`. LOCAL, not upstream. The whole
+                // cause is this change's only `effects/mod.rs` edit — the
+                // `ResolutionFrame::CipherEncode` arm added to the exhaustive
+                // `resume_resolution_frames` dispatch (3 comment + 3 code lines),
+                // which sits above all three producers; `git diff --numstat` against
+                // the merge base reads `6 0` for that file, with no hunk below them.
+                //
+                // Predicted before it was read, which is what makes this additivity
+                // rather than a fixup: main's `:7003/:7080/:10318` plus `+6` equals the
+                // observation on all three. The `+1` those main coordinates already
+                // carry is upstream's, adjudicated one paragraph above; this entry adds
+                // only its own `+6` on top and re-measures rather than composing the two
+                // arithmetics.
+                //
+                // Identity re-established, not assumed: sha256 of each producer line at
+                // its new coordinate against `origin/main:effects/mod.rs` at its old one
+                // gives `9869a19f28c791ee`, `2bc316e3aa0297f8`, `8df98486627bfe15` — the
+                // same three digests this log has carried since the first merge, so this
+                // is pure line movement. The `WaitingFor::OptionalEffectChoice` occurrence
+                // count in that file reads **25** on both sides, and
+                // `scoped_library_search.rs:452` and `engine.rs:12912` did NOT move: that
+                // is the set-preservation control. The new frame arms its prompt through
+                // the frame-resume dispatch, not through a minting site, so a sixth
+                // producer would have appeared here as a NEW entry rather than a shifted
+                // one.
+                "game/effects/mod.rs:7009".to_string(),
+                "game/effects/mod.rs:7086".to_string(),
+                "game/effects/mod.rs:10324".to_string(),
                 // UNMOVED across the rebase, and that is itself evidence the SET did not
                 // move: a census that had gained or lost a producer would not leave this
                 // entry both byte-identical AND at the same coordinate.

crates/engine/src/game/engine_resolution_choices.rs

@@ -6675,6 +6675,21 @@ pub(super) fn handle_resolution_choice(
             // of clobbering it with `Priority`; otherwise resolution is complete,
             // so return to priority and let the resulting zone change's triggers /
             // SBAs process.
+            // CR 702.99a: the offer's own frame owns this prompt (issue #7470),
+            // so consume it BEFORE the card moves — the encode's zone change can
+            // park frames of its own, and a stale owner underneath them would
+            // fail `validate` at the next prompt. This holds for BOTH answers:
+            // a decline (`creature: None`) ends the offer just as an acceptance
+            // does, so it must consume the owner just as an acceptance does.
+            //
+            // The error is surfaced rather than swallowed: it means some other
+            // frame is sitting on top of this prompt's owner, which is the exact
+            // corruption this frame was introduced to make impossible. `Ok(None)`
+            // is not that — it is an empty stack, i.e. no owner to leave stale,
+            // which is what a game saved before this frame existed restores as.
+            state
+                .take_active_cipher_encode_frame()
+                .map_err(|error| EngineError::InvalidAction(error.to_string()))?;
             match crate::game::cipher::handle_encode_choice(state, card_id, creature, events) {
                 crate::game::zone_pipeline::ZoneMoveResult::Done => {
                     ResolutionChoiceOutcome::WaitingFor(WaitingFor::Priority {

crates/engine/src/types/game_state.rs

@@ -19436,6 +19436,44 @@ impl GameState {
         self.resolution_stack.push_mutate_merge(pending);
     }
 
+    /// CR 702.99a: Park a Cipher encode offer as the active prompt owner.
+    pub fn push_cipher_encode_frame(&mut self, pending: super::resolution::PendingCipherEncode) {
+        self.resolution_stack.push_cipher_encode(pending);
+    }
+
+    /// CR 702.99a: Park a Cipher encode offer beneath the frame that owns the
+    /// spell's own prompt, so the encode arms only after that owner is
+    /// consumed.
+    ///
+    /// The position is the stack's decision, not this caller's: a parked offer
+    /// owns no prompt while `Parked`, and where such a frame may sit is a
+    /// property of the stack's current shape — see
+    /// [`ParkedFramePlacement`](super::resolution::ParkedFramePlacement). This
+    /// deliberately does NOT go through `InsertParentOfActive`: inserting below
+    /// the top is a structural guess that lands inside a paused
+    /// post-replacement/draw pair, and by the time the resulting `Err` came
+    /// back the caller had already retained its card off the normal resolution
+    /// route.
+    pub fn park_cipher_encode_beneath_live_prompt(
+        &mut self,
+        pending: super::resolution::PendingCipherEncode,
+    ) -> Result<(), ResolutionStackError> {
+        self.resolve_and_apply_frame_transition(ResolvedFrameTransition::ParkBeneathLivePrompt {
+            frame: super::resolution::ResolutionFrame::CipherEncode(pending),
+        })
+        .map(|_| ())
+        .map_err(|error| match error {
+            ResolvedFrameTransitionReplayInvariantError::Stack(error) => error,
+        })
+    }
+
+    /// CR 702.99a: Consume the active Cipher encode offer once answered.
+    pub fn take_active_cipher_encode_frame(
+        &mut self,
+    ) -> Result<Option<super::resolution::PendingCipherEncode>, ResolutionStackError> {
+        self.resolution_stack.take_active_cipher_encode()
+    }
+
     /// Re-parks the active mutate-merge owner without exposing an empty-stack
     /// interval.
     pub fn replace_active_mutate_merge_frame(
@@ -20282,6 +20320,9 @@ impl GameState {
             ResolvedFrameTransition::InsertParentOfActive { frame } => {
                 resolution_stack.insert_parent_of_active(frame.clone())?;
             }
+            ResolvedFrameTransition::ParkBeneathLivePrompt { frame } => {
+                let _ = resolution_stack.park_beneath_live_prompt(frame.clone());
+            }
             ResolvedFrameTransition::PopExpected { kind } => {
                 let _ = resolution_stack.pop_expected(*kind)?;
             }