From d23eed7407e96cd7d44c3ae1c3fdd4803753b6bb Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 21:30:56 -0700 Subject: [PATCH 1/4] Fix legacy trigger provenance restore --- crates/engine/src/game/game_object.rs | 33 +-- crates/engine/src/types/ability.rs | 61 ++++++ crates/engine/src/types/game_state.rs | 189 +++++++++++++++++- crates/engine/src/types/resolution.rs | 17 ++ .../unmaterialized_lki_serialization.rs | 180 +++++++++++++++++ 5 files changed, 455 insertions(+), 25 deletions(-) diff --git a/crates/engine/src/game/game_object.rs b/crates/engine/src/game/game_object.rs index e6f1d0dcf8..3acfa33013 100644 --- a/crates/engine/src/game/game_object.rs +++ b/crates/engine/src/game/game_object.rs @@ -11,6 +11,7 @@ use crate::types::ability::{ ExiledSpellRider, ModalChoice, ReplacementDefinition, SeatDirection, SolveCondition, SpellCastingOption, StaticDefinition, TriggerBaseSetInstanceRef, TriggerDefinition, TriggerDefinitionOccurrenceRef, TriggerEntry, TriggerOccurrenceState, + materialize_legacy_printed_trigger_entries, }; use crate::types::card::{LayoutKind, PrintedCardRef, PrintedLoyalty, TokenImageRef}; use crate::types::card_type::{CardType, CoreType}; @@ -1631,32 +1632,16 @@ impl GameObject { /// runtime payloads are rejected rather than guessed from equal definition /// bytes. pub fn migrate_legacy_trigger_definitions(&mut self) -> Result<(), &'static str> { - let has_legacy_entries = self.trigger_definitions.iter_all().any(|entry| { - matches!( - entry.occurrence, - TriggerDefinitionOccurrenceRef::Unmaterialized - ) - }); - if !has_legacy_entries { + let mut entries = self.trigger_definitions.iter_all().cloned().collect(); + materialize_legacy_printed_trigger_entries( + &mut entries, + self.base_trigger_definitions.as_slice(), + self.trigger_base_set_instance, + )?; + if entries == self.trigger_definitions.iter_all().cloned().collect::>() { return self.validate_trigger_definitions(); } - if self.base_trigger_definitions.is_empty() - || self.trigger_definitions.len() != self.base_trigger_definitions.len() - || !self.trigger_definitions.iter_all().all(|entry| { - matches!( - entry.occurrence, - TriggerDefinitionOccurrenceRef::Unmaterialized - ) - }) - || !self - .trigger_definitions - .iter_all() - .zip(self.base_trigger_definitions.iter()) - .all(|(entry, base)| entry.definition == *base) - { - return Err("legacy runtime trigger payload has no provable producer or base slot"); - } - self.materialize_base_trigger_definitions(); + self.trigger_definitions = entries.into(); self.validate_trigger_definitions() } diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index 9b7f4e2fcc..e460d74a79 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -22152,6 +22152,67 @@ impl TriggerEntry { } } +/// Classifies a persisted trigger list without inferring runtime provenance. +/// A list is either wholly legacy payloads or wholly identity-bearing entries; +/// a mixture cannot establish an exact occurrence mapping. +pub(crate) fn legacy_trigger_entry_list( + entries: &[TriggerEntry], +) -> Result { + let has_legacy = entries.iter().any(|entry| { + matches!( + entry.occurrence, + TriggerDefinitionOccurrenceRef::Unmaterialized + ) + }); + if has_legacy + && !entries.iter().all(|entry| { + matches!( + entry.occurrence, + TriggerDefinitionOccurrenceRef::Unmaterialized + ) + }) + { + return Err("legacy trigger list mixes payload-only and identity-bearing entries"); + } + Ok(has_legacy) +} + +/// Materializes a payload-only list only when an ordered printed base set proves +/// every slot. Runtime copied and granted triggers have no equivalent proof. +pub(crate) fn materialize_legacy_printed_trigger_entries( + entries: &mut Vec, + base_definitions: &[TriggerDefinition], + base_set: TriggerBaseSetInstanceRef, +) -> Result<(), &'static str> { + if !legacy_trigger_entry_list(entries)? { + return Ok(()); + } + if base_definitions.is_empty() + || entries.len() != base_definitions.len() + || !entries + .iter() + .zip(base_definitions) + .all(|(entry, base)| entry.definition == *base) + { + return Err("legacy runtime trigger payload has no provable producer or base slot"); + } + *entries = base_definitions + .iter() + .cloned() + .enumerate() + .map(|(printed_index, definition)| { + TriggerEntry::new( + TriggerDefinitionOccurrenceRef::Printed { + base_set, + printed_index, + }, + definition, + ) + }) + .collect(); + Ok(()) +} + #[derive(Deserialize)] #[serde(untagged)] enum TriggerEntryWire { diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 52348a570b..2b93040707 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -16,7 +16,8 @@ use super::ability::{ ModalChoice, PermanentEntryMode, PileSource, QuantityExpr, ResolvedAbility, SearchDestinationSplit, SearchSelectionConstraint, StaticCondition, TapCreaturesAggregate, TargetFilter, TargetRef, ThisWayCause, TriggerCondition, TriggerDefinition, - TriggerDefinitionRef, TriggerEntry, + TriggerBaseSetInstanceRef, TriggerDefinitionRef, TriggerEntry, + legacy_trigger_entry_list, materialize_legacy_printed_trigger_entries, }; use super::attribution::ObjectAttribution; use super::card::{CardFace, PrintedCardRef, TokenImageRef}; @@ -7968,6 +7969,192 @@ fn migrate_legacy_batched_zone_change_trigger_fired( Ok(()) } +/// Promotes legacy payload-only trigger lists carried by persisted zone-change +/// snapshots before the typed state can expose them to a serializer. A record's +/// own source context is authoritative; an absent live-record context can use +/// only the exact initial printed base set of the persisted object with the +/// same map key and object id. Journal snapshots never receive that fallback. +pub(crate) fn migrate_legacy_zone_change_trigger_provenance( + value: &mut serde_json::Value, + additional_live_event_roots: &[&str], +) -> Result<(), String> { + let state = value + .as_object_mut() + .ok_or_else(|| "persisted game state must be a JSON object".to_string())?; + let objects = state + .get("objects") + .and_then(serde_json::Value::as_object) + .cloned() + .ok_or_else(|| "persisted game state objects must be an object".to_string())?; + + for field in [ + "created_tokens_this_turn", + "sacrificed_permanents_this_turn", + "zone_changes_this_turn", + ] { + let Some(records) = state.get_mut(field) else { + continue; + }; + let records = records + .as_array_mut() + .ok_or_else(|| format!("{field} must be an array"))?; + for record in records { + migrate_persisted_zone_change_trigger_record(record, &objects, true)?; + } + } + + visit_persisted_live_zone_changed_records(state, additional_live_event_roots, &mut |record| { + migrate_persisted_zone_change_trigger_record(record, &objects, true) + })?; + + if let Some(journal) = state.get_mut("resolved_rules_journal") { + visit_persisted_journal_zone_change_trigger_records(journal, &objects)?; + } + Ok(()) +} + +fn migrate_persisted_zone_change_trigger_record( + record: &mut serde_json::Value, + objects: &serde_json::Map, + allow_object_fallback: bool, +) -> Result<(), String> { + let record = record + .as_object_mut() + .ok_or_else(|| "persisted zone-change record must be an object".to_string())?; + let object_id: ObjectId = record + .get("object_id") + .cloned() + .ok_or_else(|| "persisted zone-change record has no object id".to_string()) + .and_then(|value| serde_json::from_value(value).map_err(|error| error.to_string()))?; + let mut entries = record + .get("trigger_definitions") + .cloned() + .map(serde_json::from_value::>) + .transpose() + .map_err(|error| error.to_string())? + .unwrap_or_default(); + let has_legacy_entries = legacy_trigger_entry_list(&entries).map_err(str::to_string)?; + + let migrated = match record.get("trigger_source_context") { + Some(serde_json::Value::Null) | None => { + if !has_legacy_entries { + return Ok(()); + } + if !allow_object_fallback { + return Err( + "legacy journal zone-change record has no record-owned trigger source context" + .to_string(), + ); + } + let object_key = object_id.0.to_string(); + let object_value = objects.get(&object_key).ok_or_else(|| { + "legacy zone-change record has no same-id persisted object base set".to_string() + })?; + let object: GameObject = + serde_json::from_value(object_value.clone()).map_err(|error| error.to_string())?; + if object.id != object_id { + return Err( + "legacy zone-change record object map key does not match serialized object id" + .to_string(), + ); + } + if object.trigger_base_set_instance != TriggerBaseSetInstanceRef::INITIAL { + return Err( + "legacy zone-change record requires the initial printed trigger base set" + .to_string(), + ); + } + materialize_legacy_printed_trigger_entries( + &mut entries, + object.base_trigger_definitions.as_slice(), + TriggerBaseSetInstanceRef::INITIAL, + ) + .map_err(str::to_string)?; + entries + } + Some(context_value) => { + let context: TriggerSourceContext = serde_json::from_value(context_value.clone()) + .map_err(|error| error.to_string())?; + if context.identity.reference.object_id != object_id { + return Err( + "zone-change trigger source context object id does not match its record" + .to_string(), + ); + } + if legacy_trigger_entry_list(&context.trigger_entries).map_err(str::to_string)? { + return Err( + "zone-change trigger source context has unmaterialized trigger entries" + .to_string(), + ); + } + if entries.len() != context.trigger_entries.len() { + return Err( + "zone-change record trigger list disagrees with its source context" + .to_string(), + ); + } + if has_legacy_entries { + if !entries + .iter() + .zip(&context.trigger_entries) + .all(|(record_entry, context_entry)| { + record_entry.definition == context_entry.definition + }) + { + return Err( + "zone-change record trigger list disagrees with its source context" + .to_string(), + ); + } + context.trigger_entries + } else { + if entries != context.trigger_entries { + return Err( + "zone-change record trigger occurrence does not match its source context" + .to_string(), + ); + } + return Ok(()); + } + } + }; + + record.insert( + "trigger_definitions".to_string(), + serde_json::to_value(migrated).map_err(|error| error.to_string())?, + ); + Ok(()) +} + +/// Journal snapshots are immutable historical authority: they may use only an +/// exact record-owned context, never a current object's printed base set. +fn visit_persisted_journal_zone_change_trigger_records( + value: &mut serde_json::Value, + objects: &serde_json::Map, +) -> Result<(), String> { + match value { + serde_json::Value::Array(values) => { + for value in values { + visit_persisted_journal_zone_change_trigger_records(value, objects)?; + } + } + serde_json::Value::Object(object) => { + if let Some(record) = serialized_zone_changed_record_mut(object) { + migrate_persisted_zone_change_trigger_record(record, objects, false)?; + return Ok(()); + } + if let Some(record) = object.get_mut("zone_change_record") { + migrate_persisted_zone_change_trigger_record(record, objects, false)?; + } + for value in object.values_mut() { + visit_persisted_journal_zone_change_trigger_records(value, objects)?; + } + } + _ => {} + } + Ok(()) +} + /// CR 400.7 + CR 603.2c: a persisted `ZoneChanged` must retain the identity of /// the ledger occurrence that produced it. The live allocator writes the /// current turn and the ledger position together, but historical payloads may diff --git a/crates/engine/src/types/resolution.rs b/crates/engine/src/types/resolution.rs index 150f794c50..de061391e7 100644 --- a/crates/engine/src/types/resolution.rs +++ b/crates/engine/src/types/resolution.rs @@ -2826,6 +2826,23 @@ impl ResolutionStateWire { // both belong to `GameStateDecode`; no wire branch gets a private // `GameState` serde shortcut. GameStateDecode::prepare_resolution_wire(&mut value, decode_mode)?; + let additional_live_event_roots: &[&str] = match version { + LEGACY_RESOLUTION_STATE_WIRE_VERSION => &[ + "pending_continuation", + "pending_choose_zone_trigger_context", + "pending_optional_trigger_event", + "pending_change_zone_iteration", + "pending_batch_deliveries", + "pending_mill_deliveries", + "pending_each_player_copy_chosen", + ][..], + RESOLUTION_STATE_WIRE_VERSION => &[], + _ => unreachable!("version was validated before migration"), + }; + crate::types::game_state::migrate_legacy_zone_change_trigger_provenance( + &mut value, + additional_live_event_roots, + )?; match version { // V1 reader compatibility path: historical keys are consumed here diff --git a/crates/engine/tests/integration/unmaterialized_lki_serialization.rs b/crates/engine/tests/integration/unmaterialized_lki_serialization.rs index e86450bf28..b8e7786a9d 100644 --- a/crates/engine/tests/integration/unmaterialized_lki_serialization.rs +++ b/crates/engine/tests/integration/unmaterialized_lki_serialization.rs @@ -2,9 +2,13 @@ use engine::game::scenario::{GameRunner, GameScenario, P0}; use engine::game::triggers::process_triggers; +use engine::game::derived_views::ClientGameStateRef; use engine::types::ability::{TriggerDefinitionOccurrenceRef, TriggerEntry}; use engine::types::actions::GameAction; +use engine::types::events::GameEvent; use engine::types::game_state::WaitingFor; +use engine::types::resolution::ResolutionStateWire; +use engine::types::resolved_commands::ResolvedZoneChangeCommand; const DIES_TRIGGER: &str = "When this creature dies, create a 1/1 green Squirrel creature token."; @@ -38,6 +42,30 @@ fn assert_materialized(label: &str, entries: impl IntoIterator &mut serde_json::Value { + wire["resolved_rules_journal"]["entries"] + .as_array_mut() + .expect("journal entries serialize as an array") + .iter_mut() + .find_map(|entry| { + entry + .get_mut("command")? + .get_mut("ZoneChange")? + .get_mut("zone_change_record") + }) + .expect("fixture journal retains a zone-change command") +} + #[test] fn dies_lki_trigger_restoration_keeps_game_state_serializable() { let mut scenario = GameScenario::new(); @@ -94,3 +122,155 @@ fn dies_lki_trigger_restoration_keeps_game_state_serializable() { serde_json::to_string(state) .expect("a game state after dies-trigger LKI restoration must serialize"); } + +#[test] +fn legacy_zone_change_trigger_records_restore_before_client_serialization() { + let mut scenario = GameScenario::new(); + scenario.at_phase(engine::types::phase::Phase::PreCombatMain); + let dying = scenario + .add_creature_from_oracle(P0, "LKI Trigger Bear", 1, 1, DIES_TRIGGER) + .id(); + let mut runner = scenario.build(); + + runner + .state_mut() + .objects + .get_mut(&dying) + .expect("dying source exists") + .damage_marked = 99; + let mut events = Vec::new(); + engine::game::sba::check_state_based_actions(runner.state_mut(), &mut events); + process_triggers(runner.state_mut(), &events); + drain_to_priority(&mut runner); + + let record = runner + .state() + .zone_changes_this_turn + .iter() + .find(|record| record.object_id == dying) + .expect("dies record exists") + .clone(); + let source = record + .trigger_source_context + .as_ref() + .expect("dies record retains exact source context") + .identity + .reference; + let cause = runner + .state_mut() + .resolved_rules_journal + .begin_proposal() + .expect("fixture opens a journal proposal"); + runner + .state_mut() + .resolved_rules_journal + .record_zone_change(ResolvedZoneChangeCommand { + object: source, + resulting_incarnation: source.incarnation + 1, + from: record.from_zone.expect("dies source left a zone"), + to: record.to_zone, + destination_position: 0, + owner: record.owner, + entry_timestamp: None, + turn_zone_change_index: record.turn_zone_change_index, + zone_change_record: record.clone(), + cause, + }) + .expect("fixture journals the dies zone change"); + let pending_event = GameEvent::ZoneChanged { + object_id: dying, + from: record.from_zone, + to: record.to_zone, + record: Box::new(record), + }; + runner.state_mut().current_trigger_event = Some(pending_event.clone()); + runner.state_mut().pending_trigger_event_batch = vec![pending_event]; + + let mut wire = serde_json::to_value(ResolutionStateWire::from_game_state( + runner.state().clone(), + )) + .expect("materialized state serializes as a resolution wire"); + let state = wire.as_object_mut().expect("wire is a state object"); + let mut legacy_record = state["zone_changes_this_turn"] + .as_array() + .expect("zone-change ledger serializes as an array")[0] + .clone(); + erase_trigger_occurrences(&mut legacy_record); + state["zone_changes_this_turn"] = serde_json::Value::Array(vec![legacy_record.clone()]); + state.insert( + "created_tokens_this_turn".to_string(), + serde_json::Value::Array(vec![legacy_record.clone()]), + ); + state.insert( + "sacrificed_permanents_this_turn".to_string(), + serde_json::Value::Array(vec![legacy_record]), + ); + erase_trigger_occurrences( + &mut state["current_trigger_event"]["data"]["record"], + ); + erase_trigger_occurrences( + &mut state["pending_trigger_event_batch"][0]["data"]["record"], + ); + erase_trigger_occurrences(journal_zone_change_record_mut(&mut wire)); + + let mut context_free_journal = wire.clone(); + journal_zone_change_record_mut(&mut context_free_journal) + .as_object_mut() + .expect("journal record is an object") + .remove("trigger_source_context"); + let error = serde_json::from_value::(context_free_journal) + .expect_err("a legacy journal record must not borrow a live object's trigger base"); + assert!( + error + .to_string() + .contains("legacy journal zone-change record has no record-owned trigger source context"), + "journal migration rejects context-free legacy trigger payloads" + ); + + let mut noninitial_base = wire.clone(); + noninitial_base["zone_changes_this_turn"][0] + .as_object_mut() + .expect("ledger record is an object") + .remove("trigger_source_context"); + noninitial_base["objects"][dying.0.to_string()]["trigger_base_set_instance"] = + serde_json::Value::from(2); + let error = serde_json::from_value::(noninitial_base) + .expect_err("context-free legacy records require the initial printed base set"); + assert!( + error + .to_string() + .contains("legacy zone-change record requires the initial printed trigger base set"), + "live fallback rejects noninitial trigger base generations" + ); + + let mut malformed_context = wire.clone(); + malformed_context["zone_changes_this_turn"][0]["trigger_source_context"]["identity"] + ["reference"]["object_id"] = serde_json::Value::from(dying.0 + 1); + let error = serde_json::from_value::(malformed_context) + .expect_err("legacy records require a source context for the exact record object"); + assert!( + error + .to_string() + .contains("zone-change trigger source context object id does not match its record"), + "context identity mismatches fail closed" + ); + + let restored = serde_json::from_value::(wire) + .expect("legacy LKI trigger records restore through their exact source context") + .into_game_state(); + let client = serde_json::to_value(ClientGameStateRef::wrap(&restored, Some(P0))) + .expect("restored state serializes for the browser"); + assert!( + client["state"].get("resolved_rules_journal").is_none(), + "client serialization continues to omit the private resolved-rules journal" + ); + for records in [ + &restored.created_tokens_this_turn, + &restored.sacrificed_permanents_this_turn, + &restored.zone_changes_this_turn, + ] { + for record in records { + assert_materialized("restored ledger", record.trigger_definitions.clone()); + } + } +} From 945d364b99e249e03f231a86a593be1379af6712 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 21:43:47 -0700 Subject: [PATCH 2/4] Fix legacy trigger restore formatting --- crates/engine/src/game/game_object.rs | 22 ++++++++++------ crates/engine/src/types/ability.rs | 4 +-- crates/engine/src/types/game_state.rs | 25 ++++++++----------- .../unmaterialized_lki_serialization.rs | 23 +++++++---------- 4 files changed, 35 insertions(+), 39 deletions(-) diff --git a/crates/engine/src/game/game_object.rs b/crates/engine/src/game/game_object.rs index 3acfa33013..4b38fc56ac 100644 --- a/crates/engine/src/game/game_object.rs +++ b/crates/engine/src/game/game_object.rs @@ -5,13 +5,13 @@ use serde::{Deserialize, Serialize}; use crate::types::ability::{ additional_cost_instance_payment_count, additional_cost_instance_payment_count_for_ordinal, - AbilityBlockEntry, AbilityDefinition, AdditionalCost, AdditionalCostInstancePayment, - AdditionalCostOrigin, BasicLandType, CastTimingPermission, CastVariantPaid, CastingPermission, - CastingRestriction, ChosenAttribute, ChosenSubtypeKind, CostPaidObjectSnapshot, - ExiledSpellRider, ModalChoice, ReplacementDefinition, SeatDirection, SolveCondition, - SpellCastingOption, StaticDefinition, TriggerBaseSetInstanceRef, TriggerDefinition, - TriggerDefinitionOccurrenceRef, TriggerEntry, TriggerOccurrenceState, - materialize_legacy_printed_trigger_entries, + materialize_legacy_printed_trigger_entries, AbilityBlockEntry, AbilityDefinition, + AdditionalCost, AdditionalCostInstancePayment, AdditionalCostOrigin, BasicLandType, + CastTimingPermission, CastVariantPaid, CastingPermission, CastingRestriction, ChosenAttribute, + ChosenSubtypeKind, CostPaidObjectSnapshot, ExiledSpellRider, ModalChoice, + ReplacementDefinition, SeatDirection, SolveCondition, SpellCastingOption, StaticDefinition, + TriggerBaseSetInstanceRef, TriggerDefinition, TriggerDefinitionOccurrenceRef, TriggerEntry, + TriggerOccurrenceState, }; use crate::types::card::{LayoutKind, PrintedCardRef, PrintedLoyalty, TokenImageRef}; use crate::types::card_type::{CardType, CoreType}; @@ -1638,7 +1638,13 @@ impl GameObject { self.base_trigger_definitions.as_slice(), self.trigger_base_set_instance, )?; - if entries == self.trigger_definitions.iter_all().cloned().collect::>() { + if entries + == self + .trigger_definitions + .iter_all() + .cloned() + .collect::>() + { return self.validate_trigger_definitions(); } self.trigger_definitions = entries.into(); diff --git a/crates/engine/src/types/ability.rs b/crates/engine/src/types/ability.rs index e460d74a79..d9ade804f7 100644 --- a/crates/engine/src/types/ability.rs +++ b/crates/engine/src/types/ability.rs @@ -22155,9 +22155,7 @@ impl TriggerEntry { /// Classifies a persisted trigger list without inferring runtime provenance. /// A list is either wholly legacy payloads or wholly identity-bearing entries; /// a mixture cannot establish an exact occurrence mapping. -pub(crate) fn legacy_trigger_entry_list( - entries: &[TriggerEntry], -) -> Result { +pub(crate) fn legacy_trigger_entry_list(entries: &[TriggerEntry]) -> Result { let has_legacy = entries.iter().any(|entry| { matches!( entry.occurrence, diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 2b93040707..9c5c370f89 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -6,7 +6,8 @@ use rand_chacha::ChaCha20Rng; use serde::{Deserialize, Serialize}; use super::ability::{ - default_target_filter_permanent, AbilityCost, AbilityDefinition, AdditionalCost, + default_target_filter_permanent, legacy_trigger_entry_list, + materialize_legacy_printed_trigger_entries, AbilityCost, AbilityDefinition, AdditionalCost, AdditionalCostInstance, AdditionalCostInstancePayment, AttackSubject, BeholdCostAction, CastTimingPermission, CastVariantPaid, CategoryChooserScope, ChoiceType, ChoiceValue, ChooseFromZoneConstraint, ChosenAttribute, CoinFlipResult, Comparator, ContinuousModification, @@ -15,9 +16,8 @@ use super::ability::{ EffectKind, FaceDownProfile, GameRestriction, KeywordAction, KickerVariant, LibraryPosition, ModalChoice, PermanentEntryMode, PileSource, QuantityExpr, ResolvedAbility, SearchDestinationSplit, SearchSelectionConstraint, StaticCondition, TapCreaturesAggregate, - TargetFilter, TargetRef, ThisWayCause, TriggerCondition, TriggerDefinition, - TriggerBaseSetInstanceRef, TriggerDefinitionRef, TriggerEntry, - legacy_trigger_entry_list, materialize_legacy_printed_trigger_entries, + TargetFilter, TargetRef, ThisWayCause, TriggerBaseSetInstanceRef, TriggerCondition, + TriggerDefinition, TriggerDefinitionRef, TriggerEntry, }; use super::attribution::ObjectAttribution; use super::card::{CardFace, PrintedCardRef, TokenImageRef}; @@ -8073,8 +8073,8 @@ fn migrate_persisted_zone_change_trigger_record( entries } Some(context_value) => { - let context: TriggerSourceContext = serde_json::from_value(context_value.clone()) - .map_err(|error| error.to_string())?; + let context: TriggerSourceContext = + serde_json::from_value(context_value.clone()).map_err(|error| error.to_string())?; if context.identity.reference.object_id != object_id { return Err( "zone-change trigger source context object id does not match its record" @@ -8089,18 +8089,15 @@ fn migrate_persisted_zone_change_trigger_record( } if entries.len() != context.trigger_entries.len() { return Err( - "zone-change record trigger list disagrees with its source context" - .to_string(), + "zone-change record trigger list disagrees with its source context".to_string(), ); } if has_legacy_entries { - if !entries - .iter() - .zip(&context.trigger_entries) - .all(|(record_entry, context_entry)| { + if !entries.iter().zip(&context.trigger_entries).all( + |(record_entry, context_entry)| { record_entry.definition == context_entry.definition - }) - { + }, + ) { return Err( "zone-change record trigger list disagrees with its source context" .to_string(), diff --git a/crates/engine/tests/integration/unmaterialized_lki_serialization.rs b/crates/engine/tests/integration/unmaterialized_lki_serialization.rs index b8e7786a9d..a4f472000c 100644 --- a/crates/engine/tests/integration/unmaterialized_lki_serialization.rs +++ b/crates/engine/tests/integration/unmaterialized_lki_serialization.rs @@ -1,8 +1,8 @@ //! A dies-trigger LKI restoration must leave only serializable trigger entries. +use engine::game::derived_views::ClientGameStateRef; use engine::game::scenario::{GameRunner, GameScenario, P0}; use engine::game::triggers::process_triggers; -use engine::game::derived_views::ClientGameStateRef; use engine::types::ability::{TriggerDefinitionOccurrenceRef, TriggerEntry}; use engine::types::actions::GameAction; use engine::types::events::GameEvent; @@ -186,10 +186,9 @@ fn legacy_zone_change_trigger_records_restore_before_client_serialization() { runner.state_mut().current_trigger_event = Some(pending_event.clone()); runner.state_mut().pending_trigger_event_batch = vec![pending_event]; - let mut wire = serde_json::to_value(ResolutionStateWire::from_game_state( - runner.state().clone(), - )) - .expect("materialized state serializes as a resolution wire"); + let mut wire = + serde_json::to_value(ResolutionStateWire::from_game_state(runner.state().clone())) + .expect("materialized state serializes as a resolution wire"); let state = wire.as_object_mut().expect("wire is a state object"); let mut legacy_record = state["zone_changes_this_turn"] .as_array() @@ -205,12 +204,8 @@ fn legacy_zone_change_trigger_records_restore_before_client_serialization() { "sacrificed_permanents_this_turn".to_string(), serde_json::Value::Array(vec![legacy_record]), ); - erase_trigger_occurrences( - &mut state["current_trigger_event"]["data"]["record"], - ); - erase_trigger_occurrences( - &mut state["pending_trigger_event_batch"][0]["data"]["record"], - ); + erase_trigger_occurrences(&mut state["current_trigger_event"]["data"]["record"]); + erase_trigger_occurrences(&mut state["pending_trigger_event_batch"][0]["data"]["record"]); erase_trigger_occurrences(journal_zone_change_record_mut(&mut wire)); let mut context_free_journal = wire.clone(); @@ -221,9 +216,9 @@ fn legacy_zone_change_trigger_records_restore_before_client_serialization() { let error = serde_json::from_value::(context_free_journal) .expect_err("a legacy journal record must not borrow a live object's trigger base"); assert!( - error - .to_string() - .contains("legacy journal zone-change record has no record-owned trigger source context"), + error.to_string().contains( + "legacy journal zone-change record has no record-owned trigger source context" + ), "journal migration rejects context-free legacy trigger payloads" ); From 050d433134df81b80dff1b141f55870a9b040c37 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Thu, 13 Aug 2026 22:00:15 -0700 Subject: [PATCH 3/4] Harden legacy trigger restoration review coverage --- crates/engine/src/types/game_state.rs | 158 +++++++++++++----- crates/engine/src/types/resolution.rs | 89 +++++----- .../unmaterialized_lki_serialization.rs | 24 ++- 3 files changed, 188 insertions(+), 83 deletions(-) diff --git a/crates/engine/src/types/game_state.rs b/crates/engine/src/types/game_state.rs index 9c5c370f89..8f9dc2dae3 100644 --- a/crates/engine/src/types/game_state.rs +++ b/crates/engine/src/types/game_state.rs @@ -1,3 +1,4 @@ +use std::collections::hash_map::Entry; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque}; use std::sync::Arc; @@ -7969,11 +7970,13 @@ fn migrate_legacy_batched_zone_change_trigger_fired( Ok(()) } -/// Promotes legacy payload-only trigger lists carried by persisted zone-change -/// snapshots before the typed state can expose them to a serializer. A record's -/// own source context is authoritative; an absent live-record context can use -/// only the exact initial printed base set of the persisted object with the -/// same map key and object id. Journal snapshots never receive that fallback. +/// CR 400.7 + CR 603.10a: zone-change triggers use the exact source at the +/// event, rather than a same-id object from a later zone change. Promotes +/// legacy payload-only trigger lists carried by persisted zone-change snapshots +/// before the typed state can expose them to a serializer. A record's own +/// source context is authoritative; an absent live-record context can use only +/// the exact initial printed base set of the persisted object with the same map +/// key and object id. Journal snapshots never receive that fallback. pub(crate) fn migrate_legacy_zone_change_trigger_provenance( value: &mut serde_json::Value, additional_live_event_roots: &[&str], @@ -7981,41 +7984,93 @@ pub(crate) fn migrate_legacy_zone_change_trigger_provenance( let state = value .as_object_mut() .ok_or_else(|| "persisted game state must be a JSON object".to_string())?; - let objects = state + if !state .get("objects") .and_then(serde_json::Value::as_object) - .cloned() - .ok_or_else(|| "persisted game state objects must be an object".to_string())?; - - for field in [ - "created_tokens_this_turn", - "sacrificed_permanents_this_turn", - "zone_changes_this_turn", - ] { - let Some(records) = state.get_mut(field) else { - continue; - }; - let records = records - .as_array_mut() - .ok_or_else(|| format!("{field} must be an array"))?; - for record in records { - migrate_persisted_zone_change_trigger_record(record, &objects, true)?; - } - } + .is_some() + { + return Err("persisted game state objects must be an object".to_string()); + } + // Keep the serialized object map in place without cloning it. Removing it + // temporarily lets the mutable record walk borrow `state` while fallback + // lookups retain an immutable view of the original persisted objects. + let objects_value = state + .remove("objects") + .expect("the checked persisted game state retains its object map"); + let migration = { + let objects = objects_value + .as_object() + .expect("the checked persisted object map remains an object"); + let mut trigger_bases = HashMap::new(); + + (|| { + for field in [ + "created_tokens_this_turn", + "sacrificed_permanents_this_turn", + "zone_changes_this_turn", + ] { + let Some(records) = state.get_mut(field) else { + continue; + }; + let records = records + .as_array_mut() + .ok_or_else(|| format!("{field} must be an array"))?; + for record in records { + migrate_persisted_zone_change_trigger_record( + record, + objects, + &mut trigger_bases, + true, + )?; + } + } - visit_persisted_live_zone_changed_records(state, additional_live_event_roots, &mut |record| { - migrate_persisted_zone_change_trigger_record(record, &objects, true) - })?; + visit_persisted_live_zone_changed_records( + state, + additional_live_event_roots, + &mut |record| { + migrate_persisted_zone_change_trigger_record( + record, + objects, + &mut trigger_bases, + true, + ) + }, + )?; - if let Some(journal) = state.get_mut("resolved_rules_journal") { - visit_persisted_journal_zone_change_trigger_records(journal, &objects)?; - } - Ok(()) + if let Some(journal) = state.get_mut("resolved_rules_journal") { + visit_persisted_journal_zone_change_trigger_records( + journal, + objects, + &mut trigger_bases, + )?; + } + Ok(()) + })() + }; + state.insert("objects".to_string(), objects_value); + migration +} + +/// The only persisted object fields that can prove a context-free legacy +/// record was one of its initial printed trigger slots. +#[derive(Deserialize)] +struct PersistedTriggerBase { + id: ObjectId, + #[serde(default = "initial_trigger_base_set_instance")] + trigger_base_set_instance: TriggerBaseSetInstanceRef, + #[serde(default)] + base_trigger_definitions: Vec, +} + +const fn initial_trigger_base_set_instance() -> TriggerBaseSetInstanceRef { + TriggerBaseSetInstanceRef::INITIAL } fn migrate_persisted_zone_change_trigger_record( record: &mut serde_json::Value, objects: &serde_json::Map, + trigger_bases: &mut HashMap, allow_object_fallback: bool, ) -> Result<(), String> { let record = record @@ -8050,8 +8105,13 @@ fn migrate_persisted_zone_change_trigger_record( let object_value = objects.get(&object_key).ok_or_else(|| { "legacy zone-change record has no same-id persisted object base set".to_string() })?; - let object: GameObject = - serde_json::from_value(object_value.clone()).map_err(|error| error.to_string())?; + let object = match trigger_bases.entry(object_id) { + Entry::Occupied(entry) => entry.into_mut(), + Entry::Vacant(entry) => entry.insert( + serde_json::from_value(object_value.clone()) + .map_err(|error| error.to_string())?, + ), + }; if object.id != object_id { return Err( "legacy zone-change record object map key does not match serialized object id" @@ -8123,28 +8183,46 @@ fn migrate_persisted_zone_change_trigger_record( Ok(()) } -/// Journal snapshots are immutable historical authority: they may use only an -/// exact record-owned context, never a current object's printed base set. +/// CR 400.7 + CR 603.10a: journal snapshots are immutable historical +/// authority. They may use only an exact record-owned context, never a current +/// object's printed base set. fn visit_persisted_journal_zone_change_trigger_records( value: &mut serde_json::Value, objects: &serde_json::Map, + trigger_bases: &mut HashMap, ) -> Result<(), String> { match value { serde_json::Value::Array(values) => { for value in values { - visit_persisted_journal_zone_change_trigger_records(value, objects)?; + visit_persisted_journal_zone_change_trigger_records(value, objects, trigger_bases)?; } } serde_json::Value::Object(object) => { if let Some(record) = serialized_zone_changed_record_mut(object) { - migrate_persisted_zone_change_trigger_record(record, objects, false)?; + migrate_persisted_zone_change_trigger_record( + record, + objects, + trigger_bases, + false, + )?; return Ok(()); } if let Some(record) = object.get_mut("zone_change_record") { - migrate_persisted_zone_change_trigger_record(record, objects, false)?; + migrate_persisted_zone_change_trigger_record( + record, + objects, + trigger_bases, + false, + )?; } - for value in object.values_mut() { - visit_persisted_journal_zone_change_trigger_records(value, objects)?; + for (key, value) in object { + if key != "zone_change_record" { + visit_persisted_journal_zone_change_trigger_records( + value, + objects, + trigger_bases, + )?; + } } } _ => {} diff --git a/crates/engine/src/types/resolution.rs b/crates/engine/src/types/resolution.rs index de061391e7..211bef18d2 100644 --- a/crates/engine/src/types/resolution.rs +++ b/crates/engine/src/types/resolution.rs @@ -2747,6 +2747,19 @@ pub const RESOLUTION_STATE_WIRE_VERSION: u64 = 2; /// never emits v1 resolution fields. const LEGACY_RESOLUTION_STATE_WIRE_VERSION: u64 = 1; +/// V1 suspension carriers that may retain an active `GameEvent::ZoneChanged`. +/// Both provenance materialization and occurrence-key reconciliation must visit +/// this exact legacy surface before it projects into the v2 frame stack. +const LEGACY_LIVE_ZONE_CHANGED_EVENT_ROOTS: &[&str] = &[ + "pending_continuation", + "pending_choose_zone_trigger_context", + "pending_optional_trigger_event", + "pending_change_zone_iteration", + "pending_batch_deliveries", + "pending_mill_deliveries", + "pending_each_player_copy_chosen", +]; + /// Versioned wire adapter for full game-state persistence and transport. /// /// This adapter is the persistence seam between v1's legacy-only payloads and @@ -2826,47 +2839,35 @@ impl ResolutionStateWire { // both belong to `GameStateDecode`; no wire branch gets a private // `GameState` serde shortcut. GameStateDecode::prepare_resolution_wire(&mut value, decode_mode)?; - let additional_live_event_roots: &[&str] = match version { - LEGACY_RESOLUTION_STATE_WIRE_VERSION => &[ - "pending_continuation", - "pending_choose_zone_trigger_context", - "pending_optional_trigger_event", - "pending_change_zone_iteration", - "pending_batch_deliveries", - "pending_mill_deliveries", - "pending_each_player_copy_chosen", - ][..], - RESOLUTION_STATE_WIRE_VERSION => &[], - _ => unreachable!("version was validated before migration"), + let additional_live_event_roots = match decode_mode { + GameStateDecodeMode::ResolutionWireV1 => LEGACY_LIVE_ZONE_CHANGED_EVENT_ROOTS, + GameStateDecodeMode::ResolutionWireV2 => &[], + GameStateDecodeMode::PersistedRaw + | GameStateDecodeMode::TrustedEnvelope + | GameStateDecodeMode::DirectCurrentRaw => { + return Err("invalid resolution-state wire decode mode".to_string()); + } }; crate::types::game_state::migrate_legacy_zone_change_trigger_provenance( &mut value, additional_live_event_roots, )?; - match version { + match decode_mode { // V1 reader compatibility path: historical keys are consumed here // and projected into typed frames before runtime state is restored. - LEGACY_RESOLUTION_STATE_WIRE_VERSION => { + GameStateDecodeMode::ResolutionWireV1 => { crate::types::game_state::reconcile_persisted_zone_change_occurrences( &mut value, - &[ - "pending_continuation", - "pending_choose_zone_trigger_context", - "pending_optional_trigger_event", - // These v1 frame payloads retain ZoneChanged events in - // their logical-owner, delivery, or trigger context. - "pending_change_zone_iteration", - "pending_batch_deliveries", - "pending_mill_deliveries", - "pending_each_player_copy_chosen", - ], + LEGACY_LIVE_ZONE_CHANGED_EVENT_ROOTS, )?; let object = value .as_object() .expect("the checked resolution state wire remains an object"); if object.contains_key("resolution_frames") { - return Err("v1 resolution state must not contain resolution_frames".to_string()); + return Err( + "v1 resolution state must not contain resolution_frames".to_string() + ); } if object.contains_key("resolution_stack") { return Err("v1 resolution state must not contain resolution_stack".to_string()); @@ -2896,9 +2897,7 @@ impl ResolutionStateWire { let legacy_mutate_merge = LegacyMutateMergeWire::from_value(&value)?; let legacy_replacement_tails = LegacyReplacementTailsWire::from_value(&value)?; let mut legacy_value = value; - let legacy_object = legacy_value - .as_object_mut() - .expect("checked JSON object"); + let legacy_object = legacy_value.as_object_mut().expect("checked JSON object"); legacy_object.remove("pending_continuation"); legacy_object.remove("search_continuation_attach_host"); legacy_object.remove("pending_choose_zone_trigger_context"); @@ -3027,7 +3026,7 @@ impl ResolutionStateWire { debug_assert_runtime_resolution_invariants(&legacy); Ok(Self { state: legacy }) } - RESOLUTION_STATE_WIRE_VERSION => { + GameStateDecodeMode::ResolutionWireV2 => { crate::types::game_state::reconcile_persisted_zone_change_occurrences( &mut value, &[], @@ -3036,11 +3035,14 @@ impl ResolutionStateWire { .as_object() .expect("the checked resolution state wire remains an object"); if legacy_resolution_wire_field(object).is_some() { - return Err("v2 resolution state must not contain a legacy resolution field".to_string()); + return Err( + "v2 resolution state must not contain a legacy resolution field" + .to_string(), + ); } - let frames_value = object - .get("resolution_frames") - .ok_or_else(|| "v2 resolution state is missing resolution_frames".to_string())?; + let frames_value = object.get("resolution_frames").ok_or_else(|| { + "v2 resolution state is missing resolution_frames".to_string() + })?; if has_removed_batched_repeated_optional_payment(frames_value) { return Err( "v2 repeated optional-payment snapshot uses removed batched:true flow; restart the game from a current save" @@ -3057,8 +3059,9 @@ impl ResolutionStateWire { state_object.remove("resolution_state_version"); state_object.remove("resolution_frames"); if state_object.remove("resolution_stack").is_some() { - return Err("v2 resolution state must not contain runtime resolution_stack" - .to_string()); + return Err( + "v2 resolution state must not contain runtime resolution_stack".to_string(), + ); } let state = GameStateDecode::materialize_prepared(state_value)?; frames @@ -3067,17 +3070,21 @@ impl ResolutionStateWire { let projected = project_frames_into_legacy_state(&state, &frames)?; let canonical = canonicalize_legacy_resolution_state(&projected)?; if canonical != frames { - return Err("v2 resolution frames cannot be represented by the legacy runtime slots" - .to_string()); + return Err( + "v2 resolution frames cannot be represented by the legacy runtime slots" + .to_string(), + ); } crate::types::game_state::validate_trigger_firing_coherence(&projected)?; #[cfg(debug_assertions)] debug_assert_runtime_resolution_invariants(&projected); Ok(Self { state: projected }) } - other => Err(format!( - "unsupported resolution_state_version {other}; expected 1 or {RESOLUTION_STATE_WIRE_VERSION}" - )), + GameStateDecodeMode::PersistedRaw + | GameStateDecodeMode::TrustedEnvelope + | GameStateDecodeMode::DirectCurrentRaw => { + Err("invalid resolution-state wire decode mode".to_string()) + } } } } diff --git a/crates/engine/tests/integration/unmaterialized_lki_serialization.rs b/crates/engine/tests/integration/unmaterialized_lki_serialization.rs index a4f472000c..8a2af55e55 100644 --- a/crates/engine/tests/integration/unmaterialized_lki_serialization.rs +++ b/crates/engine/tests/integration/unmaterialized_lki_serialization.rs @@ -43,9 +43,14 @@ fn assert_materialized(label: &str, entries: impl IntoIterator Date: Thu, 13 Aug 2026 22:17:46 -0700 Subject: [PATCH 4/4] Fix resolution boundary guard for v1 reader --- scripts/check-resolution-frame-boundaries.sh | 21 ++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/scripts/check-resolution-frame-boundaries.sh b/scripts/check-resolution-frame-boundaries.sh index 466a00855a..3d568a60db 100755 --- a/scripts/check-resolution-frame-boundaries.sh +++ b/scripts/check-resolution-frame-boundaries.sh @@ -208,7 +208,10 @@ def line_number(source: str, offset: int) -> int: def legacy_allowlist_spans(source: str) -> list[tuple[int, int]]: - v1_match = re.search(r"\bLEGACY_RESOLUTION_STATE_WIRE_VERSION\s*=>\s*\{", source) + # The version discriminator maps numeric wire versions to the typed decode + # mode before the reader match. Anchor the allowlist to that typed v1 arm, + # which is the sole branch that consumes legacy resolution fields. + v1_match = re.search(r"\bGameStateDecodeMode::ResolutionWireV1\s*=>\s*\{", source) if v1_match is None: raise ValueError("missing ResolutionStateWire v1 reader arm") @@ -216,7 +219,21 @@ def legacy_allowlist_spans(source: str) -> list[tuple[int, int]]: if inventory_match is None: raise ValueError("missing legacy resolution key inventory") - spans = [block_span(source, v1_match), block_span(source, inventory_match)] + live_roots_match = re.search( + r"\bconst\s+LEGACY_LIVE_ZONE_CHANGED_EVENT_ROOTS\s*:\s*&\[&str\]\s*=\s*&\[", + source, + ) + if live_roots_match is None: + raise ValueError("missing legacy live ZoneChanged root census") + live_roots_end = source.find("];", live_roots_match.end()) + if live_roots_end == -1: + raise ValueError("unterminated legacy live ZoneChanged root census") + + spans = [ + block_span(source, v1_match), + block_span(source, inventory_match), + (live_roots_match.start(), live_roots_end + 2), + ] legacy_struct = re.compile(r"\bstruct\s+Legacy\w+Wire\b[^\{]*\{") spans.extend(block_span(source, match) for match in legacy_struct.finditer(source)) return spans