fix(engine): auto-reattach Gift of Immortality at next end step

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

@@ -63,12 +63,30 @@ fn resolve_forward_result_search_attach_host(
     let targets = forward_result_attach_host_targets(ability);
     match target {
         TargetFilter::SelfRef => Some(AttachTarget::Object(ability.source_id)),
+        // CR 303.4b + CR 109.4: "attached to you" (Lynde, Cheerful Tormentor)
+        // binds the Aura host to the resolving ability's controller.
+        TargetFilter::Controller => Some(AttachTarget::Player(ability.controller)),
         TargetFilter::ParentTarget => targets
             .first()
             .map(|target| match target {
                 TargetRef::Object(id) => AttachTarget::Object(*id),
                 TargetRef::Player(id) => AttachTarget::Player(*id),
             })
+            .or_else(|| {
+                // CR 603.2 + CR 608.2c + CR 701.3a: Event-subject return Auras
+                // ("return this … attached to that creature" — Dragon Breath,
+                // Smoke Shroud) bind ParentTarget to the trigger-event referent
+                // (entering creature), not the Aura's prior host.
+                crate::game::targeting::resolve_event_context_target(
+                    state,
+                    &TargetFilter::ParentTarget,
+                    ability.source_id,
+                )
+                .map(|target| match target {
+                    TargetRef::Object(id) => AttachTarget::Object(id),
+                    TargetRef::Player(id) => AttachTarget::Player(id),
+                })
+            })
             .or_else(|| {
                 // CR 303.4b + CR 608.2c: Aura search-put "attached to that/enchanted
                 // player" binds ParentTarget to the source's enchanted host when no

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

@@ -44,8 +44,19 @@ pub fn resolve(
     // chosen object. This ordering is mandatory: running the contextual bind
     // first would pre-empt the tracked-set rewrite and break the "those cards"
     // cards.
+    // CR 603.7: Prefer the active nonempty resolution-chain set, then the latest
+    // nonempty published set. An empty chain id (stale pre-choice publish) must
+    // not shadow a later nonempty set (Storm Herald delayed exile).
     let tracked_set_id = if uses_tracked_set {
-        crate::game::targeting::latest_tracked_set_id(state)
+        state
+            .chain_tracked_set_id
+            .filter(|id| {
+                state
+                    .tracked_object_sets
+                    .get(id)
+                    .is_some_and(|objects| !objects.is_empty())
+            })
+            .or_else(|| crate::game::targeting::latest_tracked_set_id(state))
     } else {
         None
     };
@@ -127,7 +138,7 @@ pub fn resolve(
         )
         .map(|t| vec![t])
         .unwrap_or_default()
-    } else if super::effect_refs_parent_target(&delayed_ability.effect) {
+    } else if super::ability_refs_parent_target(&delayed_ability) {
         parent_target_snapshot(state, ability)
     } else if effect_references_last_created(&delayed_ability.effect)
         && !state.last_created_token_ids.is_empty()
@@ -170,13 +181,28 @@ pub fn resolve(
     // and activated-ability sources may not already carry trigger provenance;
     // capture their current incarnation at creation rather than later rebinding
     // the stored ObjectId. CR 400.7.
+    //
+    // CR 400.7 + CR 603.7c: when the delayed ability's source is still the same
+    // ObjectId, refresh zone + incarnation to that object's creation-time
+    // location. The parent trigger often latched while the source was on the
+    // battlefield (Gift of Immortality's dies trigger), but SBAs may already
+    // have moved that Aura to the graveyard (bumping incarnation) before the
+    // delayed return is created. SelfRef resolution requires a zone+incarnation
+    // match (`source_is_current_via_zone_match`); keeping the BF/pre-move stamp
+    // would make the end-step SelfRef ChangeZone no-op.
     let source_context = ability.trigger_source.clone().or_else(|| {
         state
             .objects
             .get(&ability.source_id)
             .map(|source| super::super::triggers::trigger_source_context_for_latch(state, source))
     });
-    if let Some(source_context) = source_context {
+    if let Some(mut source_context) = source_context {
+        if source_context.identity.reference.object_id == delayed_ability.source_id {
+            if let Some(obj) = state.objects.get(&delayed_ability.source_id) {
+                source_context.identity.expected_zone = obj.zone;
+                source_context.identity.reference.incarnation = obj.incarnation;
+            }
+        }
         delayed_ability.set_trigger_source_recursive(source_context);
     }
 

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

@@ -2479,14 +2479,39 @@ fn apply_parent_chain_context(
 fn should_propagate_parent_targets(ability: &ResolvedAbility, sub: &ResolvedAbility) -> bool {
     sub.targets.is_empty()
         && !ability.targets.is_empty()
-        && sub.target_choice_timing != TargetChoiceTiming::Resolution
+        && (sub.target_choice_timing != TargetChoiceTiming::Resolution
+            // CR 608.2c + CR 303.4f: TargetOnly → ChangeZone[+Attach ParentTarget]
+            // (Necrotic Plague) stamps Resolution on the return clause, but the
+            // chosen host is still the parent's bound target — propagate it so
+            // nested Attach does not fall through to the trigger-event referent.
+            || change_zone_forwards_chosen_attach_host(sub))
         && !(sub
             .effect
             .target_filter()
             .is_some_and(TargetFilter::references_exiled_by_source)
             && !effect_refs_parent_target(&sub.effect))
 }
 
+/// CR 701.3a + CR 303.4f: `forward_result` ChangeZone nesting Attach→ParentTarget
+/// carries the parent's chosen host (not a fresh Resolution-time object pick).
+fn change_zone_forwards_chosen_attach_host(sub: &ResolvedAbility) -> bool {
+    sub.forward_result
+        && matches!(
+            &sub.effect,
+            Effect::ChangeZone {
+                destination: Zone::Battlefield,
+                ..
+            }
+        )
+        && matches!(
+            sub.sub_ability.as_deref().map(|s| &s.effect),
+            Some(Effect::Attach {
+                target: TargetFilter::ParentTarget,
+                ..
+            })
+        )
+}
+
 fn waits_for_resolution_choice(waiting_for: &WaitingFor) -> bool {
     matches!(
         waiting_for,
@@ -5274,6 +5299,14 @@ fn mandatory_parent_effect_performed(effect: &Effect, events: &[GameEvent]) -> b
 }
 
 pub(crate) fn publish_tracked_set(state: &mut GameState, affected_ids: Vec<ObjectId>) {
+    // CR 603.7 + CR 608.2c: An empty publish must not allocate a chain set id —
+    // otherwise a deferred Resolution-time zone choice (Storm Herald: choose
+    // Auras, then move) can stamp `chain_tracked_set_id` to an empty set before
+    // the real move, and a following CreateDelayedTrigger binds TrackedSet to
+    // that empty id forever.
+    if affected_ids.is_empty() {
+        return;
+    }
     // CR 603.7 + CR 608.2c: Chain unification. If an ancestor in this
     // resolution chain already published a tracked set, extend that set with
     // the current publish so compound zone-changing effects expose every
@@ -5553,6 +5586,22 @@ fn ability_refs_triggering_source(ability: &ResolvedAbility) -> bool {
             .is_some_and(ability_refs_triggering_source)
 }
 
+/// True when any effect in the ability chain references `ParentTarget`
+/// (including nested sub/else abilities). Used by delayed-trigger snapshotting
+/// so an Attach host on a ChangeZone sub-chain (Gift of Immortality #4956) still
+/// freezes the parent referent at creation time.
+fn ability_refs_parent_target(ability: &ResolvedAbility) -> bool {
+    effect_refs_parent_target(&ability.effect)
+        || ability
+            .sub_ability
+            .as_deref()
+            .is_some_and(ability_refs_parent_target)
+        || ability
+            .else_ability
+            .as_deref()
+            .is_some_and(ability_refs_parent_target)
+}
+
 /// CR 603.7 + CR 109.5: Replace the first `TargetRef::Object` in a target
 /// slice with the supplied object id. Used by the `repeat_for: TrackedSetSize`
 /// per-iteration rebind so the i-th iteration's parent reference (e.g.,
@@ -6472,26 +6521,86 @@ fn filter_refs_post_replacement_event_target(filter: &TargetFilter) -> bool {
 /// effect carries an event-context recipient (`TriggeringPlayer`, etc.), bind
 /// that referent into `targets` before payer/effect resolution. Shared by the
 /// unless-pay interceptor and the main effect path (issue #2361).
+///
+/// CR 603.2 + CR 701.3a + CR 303.4f: Event-subject return Auras nest
+/// `Attach { target: ParentTarget }` under a `forward_result` ChangeZone whose
+/// own target is `SelfRef` (Dragon Breath, Smoke Shroud). Hydrate the nested
+/// host onto the Attach sub so zone entry binds the trigger-event creature
+/// instead of falling through to CR 303.4f Aura choice.
+///
+/// Do **not** inject an event referent when the ability chain already carries a
+/// bound parent target (Necrotic Plague: controller's chosen creature wins over
+/// the creature that died). `forward_result_attach_host_targets` prefers a
+/// filled Attach sub over `ability.targets`, so an eager hydrate would overwrite
+/// the explicit choice.
 fn hydrate_event_context_targets<'a>(
     state: &GameState,
     ability: &'a ResolvedAbility,
 ) -> Cow<'a, ResolvedAbility> {
-    if !ability.targets.is_empty() {
-        return Cow::Borrowed(ability);
-    }
-    let Some(filter) = extract_event_context_filter(&ability.effect) else {
-        return Cow::Borrowed(ability);
+    let root_ref = if ability.targets.is_empty() {
+        extract_event_context_filter(&ability.effect).and_then(|filter| {
+            crate::game::targeting::resolve_event_context_target(state, filter, ability.source_id)
+        })
+    } else {
+        None
     };
-    let Some(target_ref) =
-        crate::game::targeting::resolve_event_context_target(state, filter, ability.source_id)
-    else {
-        return Cow::Borrowed(ability);
+    // Only hydrate an event-context Attach host when the chain has no bound
+    // parent target yet — an explicit/snapshotted choice must stand.
+    let nested_host_ref = if ability.targets.is_empty() {
+        nested_forward_result_attach_parent_target_ref(state, ability)
+    } else {
+        None
     };
+
+    if root_ref.is_none() && nested_host_ref.is_none() {
+        return Cow::Borrowed(ability);
+    }
+
     let mut resolved = ability.clone();
-    resolved.targets = vec![target_ref];
+    if let Some(target_ref) = root_ref {
+        resolved.targets = vec![target_ref];
+    }
+    if let Some(host_ref) = nested_host_ref {
+        if let Some(sub) = resolved.sub_ability.as_mut() {
+            sub.targets = vec![host_ref];
+        }
+    }
     Cow::Owned(resolved)
 }
 
+/// CR 603.2 + CR 701.3a: Resolve `ParentTarget` for a nested forward-result
+/// Attach host when the chain still has no bound parent target.
+fn nested_forward_result_attach_parent_target_ref(
+    state: &GameState,
+    ability: &ResolvedAbility,
+) -> Option<TargetRef> {
+    if !ability.forward_result {
+        return None;
+    }
+    // CR 608.2c + CR 303.4f: A chosen/snapshotted parent target on the ability
+    // (Necrotic Plague's "target creature one of their opponents controls") must
+    // not be overwritten by the trigger-event referent (the creature that died).
+    if !ability.targets.is_empty() {
+        return None;
+    }
+    let sub = ability.sub_ability.as_ref()?;
+    if !sub.targets.is_empty() {
+        return None;
+    }
+    let Effect::Attach {
+        target: TargetFilter::ParentTarget,
+        ..
+    } = &sub.effect
+    else {
+        return None;
+    };
+    crate::game::targeting::resolve_event_context_target(
+        state,
+        &TargetFilter::ParentTarget,
+        ability.source_id,
+    )
+}
+
 /// CR 603.2: Filters that auto-resolve from `state.current_trigger_event` during
 /// hydration / unless-pay payer resolution (issue #2361, Kain #1335).
 fn hydratable_event_context_filter(filter: &TargetFilter) -> bool {
@@ -9523,7 +9632,17 @@ fn resolve_chain_body(
     if next_sub_needs_tracked_set(ability) {
         let affected_with_causes =
             affected_objects_with_causes(state, ability, &ability.effect, &events[events_before..]);
-        publish_tracked_set_with_causes(state, affected_with_causes);
+        if !affected_with_causes.is_empty() {
+            publish_tracked_set_with_causes(state, affected_with_causes);
+        } else if !waits_for_resolution_choice(&state.waiting_for) {
+            // CR 603.7: A completed effect that moved nothing must still rebind
+            // an empty chain set so TrackedSet / TrackedSetSize consumers do not
+            // reuse a prior non-empty set (Fumigate with 0 creatures;
+            // ChangeZoneAll → GrantCastingPermission). Skip while a resolution
+            // choice is still open — publishing empty mid-pause would permanently
+            // bind Storm Herald's delayed exile to an empty set before Auras move.
+            publish_fresh_tracked_set(state, Vec::new());
+        }
     }
 
     // ExileFromTopUntil handles its own sub_ability chain internally for both
@@ -10292,15 +10411,58 @@ fn resolve_chain_body(
         // isn't an implicit tracked-set consumer), prepend the moved
         // card as a target so `ParentTarget` consumers downstream
         // resolve to it.
-        if !forwarded_objects.is_empty() {
+        // CR 303.4g + CR 303.4i (#4956): `forward_result` Attach is realized via
+        // `enter_attached_to` on the zone move. If that move Remained, there is
+        // no ZoneChanged event — running Attach would stamp `attached_to` onto
+        // an Aura that never left its origin zone.
+        if ability.forward_result
+            && forwarded_objects.is_empty()
+            && matches!(
+                ability.effect,
+                Effect::ChangeZone {
+                    destination: Zone::Battlefield,
+                    ..
+                }
+            )
+            && matches!(sub.effect, Effect::Attach { .. })
+        {
+            // CR 303.4g + CR 608.2c: Zero cards returned — skip the Attach that
+            // would have bound SelfRef attachments on entry, but still run
+            // trailing instructions (Cass: "then attach any number of Equipment
+            // … to that creature"). The skipped Attach node holds the chosen
+            // host target; ParentTarget on the trailing Attach must inherit it.
+            if let Some(trailing) = sub.sub_ability.as_ref() {
+                let mut trailing_resolved = trailing.as_ref().clone();
+                if should_propagate_parent_targets(sub, &trailing_resolved) {
+                    trailing_resolved.targets = sub.targets.clone();
+                } else if should_propagate_parent_targets(ability, &trailing_resolved) {
+                    trailing_resolved.targets = ability.targets.clone();
+                }
+                apply_parent_chain_context(
+                    &mut trailing_resolved,
+                    ability,
+                    effect_context_object.as_ref(),
+                    state,
+                );
+                resolve_ability_chain(state, &trailing_resolved, events, depth + 1)?;
+            }
+        } else if !forwarded_objects.is_empty() {
             let mut sub_with_context = sub.as_ref().clone();
             // CR 707.10: `CopySpell { SelfRef }` copies the resolving spell
             // itself (Sevinne's Reclamation, Chain cycle). `forward_result`
             // rebinding `source_id` to the just-moved permanent would make
             // `copy_spell::resolve` look up the wrong stack entry after
             // `resolve_top` has popped the spell (issue #2860).
+            //
+            // CR 603.7c + CR 201.5: `CreateDelayedTrigger` must keep the
+            // creating ability's source. SelfRef inside the delayed body means
+            // "this card" (Gift of Immortality #4956), not the just-returned
+            // host. ParentTarget anaphora still receive the moved object via
+            // `targets` below (snapshot at delayed-trigger creation).
             if !copy_spell_self_ref_keeps_resolving_spell_source(sub) {
-                sub_with_context.source_id = forwarded_objects[0];
+                if !matches!(sub.effect, Effect::CreateDelayedTrigger { .. }) {
+                    sub_with_context.source_id = forwarded_objects[0];
+                }
                 if matches!(
                     ability.effect,
                     Effect::Conjure {
@@ -10325,7 +10487,22 @@ fn resolve_chain_body(
                             ..
                         }
                     );
-                    if !attach_target_is_last_created
+                    // CR 608.2c: ParentTarget hosts inherit the parent's chosen
+                    // object (Necrotic Plague / Cass trailing Equipment). Do not
+                    // append the ability source — that would rebind the host to
+                    // the returned Aura/attachment itself.
+                    let attach_target_is_parent = matches!(
+                        &sub.effect,
+                        Effect::Attach {
+                            target: TargetFilter::ParentTarget,
+                            ..
+                        }
+                    );
+                    if attach_target_is_parent {
+                        if sub_with_context.targets.is_empty() && !ability.targets.is_empty() {
+                            sub_with_context.targets = ability.targets.clone();
+                        }
+                    } else if !attach_target_is_last_created
                         && !sub_with_context
                             .targets
                             .iter()