From b065c21d343bd7df60550b395dac2134ae00e78f Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 17:15:54 -0700 Subject: [PATCH 01/11] fix: filter uncompletable AI target proposals --- .../src/adapter/__tests__/ws-adapter.test.ts | 32 +++- client/src/adapter/types.ts | 15 ++ client/src/adapter/ws-adapter.ts | 4 +- .../game/__tests__/dispatchResolveAll.test.ts | 37 +++- client/src/game/dispatch.ts | 1 + crates/engine/src/ai_support/context.rs | 14 +- .../tests/integration/ai_decision_contract.rs | 181 +++++++++++++++++- 7 files changed, 264 insertions(+), 20 deletions(-) diff --git a/client/src/adapter/__tests__/ws-adapter.test.ts b/client/src/adapter/__tests__/ws-adapter.test.ts index 42db11fb3a..c1dc50c9a1 100644 --- a/client/src/adapter/__tests__/ws-adapter.test.ts +++ b/client/src/adapter/__tests__/ws-adapter.test.ts @@ -235,6 +235,34 @@ describe("WebSocketAdapter", () => { await expect(resultPromise).rejects.toMatchObject({ message: "batch snapshot rejected" }); }); + it("scopes the stale priority race to correlated Resolve All rejections", async () => { + const stale = adapter.resolveAll(0, [{ playerId: 1, difficulty: "Medium" }], 5); + ws.dispatchSynthetic( + "message", + JSON.stringify({ + type: "ResolveAllRejected", + data: { request_id: 1, reason: "Resolve All requires your priority" }, + }), + ); + await expect(stale).rejects.toMatchObject({ + code: "STALE_ACTION", + recoverable: false, + }); + + const rejected = adapter.resolveAll(0, [{ playerId: 1, difficulty: "Medium" }], 5); + ws.dispatchSynthetic( + "message", + JSON.stringify({ + type: "ResolveAllRejected", + data: { request_id: 2, reason: "batch snapshot rejected" }, + }), + ); + await expect(rejected).rejects.toMatchObject({ + code: "ACTION_REJECTED", + recoverable: true, + }); + }); + describe("server rewind capability (F2)", () => { it("declares the capability through the standalone type guard", () => { expect(supportsServerRewind(adapter)).toBe(true); @@ -997,13 +1025,13 @@ describe("WebSocketAdapter", () => { }); }); - it("still surfaces a non-stale server rejection as a recoverable ACTION_REJECTED", async () => { + it("keeps the Resolve All priority text actionable on an ordinary action rejection", async () => { const pending = adapter.submitAction({ type: "PassPriority" }, 0); ws.dispatchSynthetic( "message", JSON.stringify({ type: "ActionRejected", - data: { reason: "Engine error: Something genuinely wrong" }, + data: { reason: "Resolve All requires your priority" }, }), ); await expect(pending).rejects.toMatchObject({ diff --git a/client/src/adapter/types.ts b/client/src/adapter/types.ts index 05abf4a652..3871dfc3d0 100644 --- a/client/src/adapter/types.ts +++ b/client/src/adapter/types.ts @@ -3568,6 +3568,21 @@ export function actionRejectionError(reason: string): AdapterError { : new AdapterError(AdapterErrorCode.ACTION_REJECTED, reason, true); } +/** + * Classify a requester-correlated Resolve All rejection. + * + * A batch request can reach the server after priority has advanced. The server + * must reject that request, but this particular response is a stale UI race, + * not an actionable error for the requester. Keep the classification scoped to + * the Resolve All protocol frame: the same text on an ordinary action + * rejection must remain visible. + */ +export function resolveAllRejectionError(reason: string): AdapterError { + return reason === "Resolve All requires your priority" + ? new AdapterError(AdapterErrorCode.STALE_ACTION, reason, false) + : actionRejectionError(reason); +} + /** * Detect the engine's rejection of a `ReorderHand` whose order no longer names * the current hand. `apply_action` formats diff --git a/client/src/adapter/ws-adapter.ts b/client/src/adapter/ws-adapter.ts index 949e4a59c3..b74a1f155d 100644 --- a/client/src/adapter/ws-adapter.ts +++ b/client/src/adapter/ws-adapter.ts @@ -18,7 +18,7 @@ import type { FormatConfig, } from "./types"; import type { InteractionSubmission } from "./generated/interaction"; -import { AdapterError, AdapterErrorCode, EMPTY_LEGAL_ACTIONS, actionRejectionError, nextSnapshotSeq } from "./types"; +import { AdapterError, AdapterErrorCode, EMPTY_LEGAL_ACTIONS, actionRejectionError, nextSnapshotSeq, resolveAllRejectionError } from "./types"; import type { BracketDeckRequest, BracketEstimate } from "../types/bracketEstimate"; import { HandshakeError, @@ -1529,7 +1529,7 @@ export class WebSocketAdapter implements EngineAdapter { case "ResolveAllRejected": { const data = msg.data as { request_id: number; reason: string }; if (this.pendingResolveAll?.requestId === data.request_id) { - this.pendingResolveAll.reject(actionRejectionError(data.reason)); + this.pendingResolveAll.reject(resolveAllRejectionError(data.reason)); this.pendingResolveAll = null; } break; diff --git a/client/src/game/__tests__/dispatchResolveAll.test.ts b/client/src/game/__tests__/dispatchResolveAll.test.ts index 632eadb096..fe364ff8a0 100644 --- a/client/src/game/__tests__/dispatchResolveAll.test.ts +++ b/client/src/game/__tests__/dispatchResolveAll.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { BatchResolveResult, EngineSnapshot, GameState } from "../../adapter/types"; -import { nextSnapshotSeq } from "../../adapter/types"; +import { AdapterError, AdapterErrorCode, nextSnapshotSeq } from "../../adapter/types"; import { useGameStore } from "../../stores/gameStore"; import { useAppNotificationStore } from "../../stores/appToastStore"; import { usePreferencesStore } from "../../stores/preferencesStore"; @@ -191,10 +191,39 @@ describe("dispatchResolveAll progress", () => { expect(submitAction).not.toHaveBeenCalled(); }); - it("shows a server-provided Resolve All rejection without rejecting the click handler", async () => { + it("silently absorbs a stale Resolve All priority rejection without rejecting the click handler", async () => { const resolveAll = vi .fn() - .mockRejectedValue(new Error("Resolve All requires your priority")); + .mockRejectedValue( + new AdapterError( + AdapterErrorCode.STALE_ACTION, + "Resolve All requires your priority", + false, + ), + ); + const getState = vi.fn().mockResolvedValue(stateWithStack(2)); + useGameStore.setState({ + gameState: stateWithStack(2), + adapter: { + resolveAll, + resolveAllUsesServerAi: true, + getState, + getLegalActions: vi.fn().mockResolvedValue({ actions: [], autoPassRecommended: false }), + getSnapshot: snapshotVia(getState), + } as never, + }); + + await expect(dispatchResolveAll(0, [])).resolves.toBeUndefined(); + + expect(useAppNotificationStore.getState().notification).toBeNull(); + expect(useGameStore.getState().isResolvingAll).toBe(false); + expect(useGameStore.getState().resolutionProgress).toBeNull(); + }); + + it("still surfaces a non-stale Resolve All rejection", async () => { + const resolveAll = vi + .fn() + .mockRejectedValue(new Error("batch snapshot rejected")); const getState = vi.fn().mockResolvedValue(stateWithStack(2)); useGameStore.setState({ gameState: stateWithStack(2), @@ -210,7 +239,7 @@ describe("dispatchResolveAll progress", () => { await expect(dispatchResolveAll(0, [])).resolves.toBeUndefined(); expect(useAppNotificationStore.getState().notification).toMatchObject({ - description: "Resolve All requires your priority", + description: "batch snapshot rejected", }); }); diff --git a/client/src/game/dispatch.ts b/client/src/game/dispatch.ts index f79447d003..74b272d087 100644 --- a/client/src/game/dispatch.ts +++ b/client/src/game/dispatch.ts @@ -1080,6 +1080,7 @@ export async function dispatchResolveAll( await saveAuthoritativeGame(gameId, adapter, newState); } } catch (err) { + if (isStaleAction(err)) return; debugLog(`Resolve All error: ${err instanceof Error ? err.message : String(err)}`); showActionError({ type: "PassPriority" }, err); } finally { diff --git a/crates/engine/src/ai_support/context.rs b/crates/engine/src/ai_support/context.rs index 5c28a489e6..6656544b4d 100644 --- a/crates/engine/src/ai_support/context.rs +++ b/crates/engine/src/ai_support/context.rs @@ -104,8 +104,6 @@ impl AiDecisionContract { pub(crate) fn target_selection_requires_reducer_validation(state: &GameState) -> bool { let WaitingFor::TargetSelection { - player, - pending_cast, target_slots, selection, .. @@ -114,15 +112,11 @@ pub(crate) fn target_selection_requires_reducer_validation(state: &GameState) -> return false; }; - // Only the final target can lock a target-dependent cost. Earlier - // selections are valid reducer continuations regardless of whether the - // eventual cost is payable. + // CR 601.2c + CR 601.2e-h + CR 602.2b: completing the final target + // selection can immediately check legality and pay the proposed spell or + // activation cost. Earlier selections cannot complete that transition, but + // every final candidate must be reducer-validated before the AI receives it. selection.current_slot.checked_add(1) == Some(target_slots.len()) - && !crate::game::casting::pending_mana_obligation_is_stable_before_targets( - state, - *player, - pending_cast, - ) } /// Whether a decision can alter the target requirements of an in-progress cast. diff --git a/crates/engine/tests/integration/ai_decision_contract.rs b/crates/engine/tests/integration/ai_decision_contract.rs index 2beb1050c0..0ec6d9ce00 100644 --- a/crates/engine/tests/integration/ai_decision_contract.rs +++ b/crates/engine/tests/integration/ai_decision_contract.rs @@ -1,20 +1,197 @@ use engine::ai_support::AiDecisionContract; use engine::game::engine::{apply_as_current, EngineError}; +use engine::game::scenario::{GameScenario, P0, P1}; use engine::game::zones::create_object; use engine::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AdditionalCost, - AdditionalCostRepeatability, Effect, TargetFilter, TypeFilter, TypedFilter, + AdditionalCostRepeatability, Effect, QuantityExpr, TargetFilter, TargetRef, TypeFilter, + TypedFilter, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; use engine::types::game_state::{CastPaymentMode, GameState, WaitingFor}; use engine::types::identifiers::{CardId, ObjectId}; -use engine::types::mana::{ManaCost, ManaType, ManaUnit}; +use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; use engine::types::phase::Phase; use engine::types::player::PlayerId; use engine::types::zones::Zone; use std::sync::Arc; +const ROUSING_REFRAIN_ORACLE: &str = "Add {R} for each card in target opponent's hand. Until end of turn, you don't lose this mana as steps and phases end. Exile Rousing Refrain with three time counters on it."; + +fn rousing_refrain_final_target_state(payable: bool) -> (GameState, Vec) { + let p3 = PlayerId(3); + let mut scenario = GameScenario::new_n_player(4, 7114); + scenario.at_phase(Phase::PreCombatMain); + for player in [PlayerId(0), PlayerId(1), PlayerId(2)] { + scenario.with_cards_in_hand(player, &["Opponent card"]); + } + let spell = scenario + .add_spell_to_hand_from_oracle(p3, "Rousing Refrain", false, ROUSING_REFRAIN_ORACLE) + .with_mana_cost(ManaCost::Cost { + shards: vec![ManaCostShard::Red, ManaCostShard::Red], + generic: 3, + }) + .id(); + if payable { + scenario.with_mana_pool( + p3, + (0..5) + .map(|_| ManaUnit::new(ManaType::Red, ObjectId(0), false, vec![])) + .collect(), + ); + } + + let mut state = scenario.build().state().clone(); + state.active_player = p3; + state.priority_player = p3; + state.waiting_for = WaitingFor::Priority { player: p3 }; + apply_as_current( + &mut state, + GameAction::CastSpell { + object_id: spell, + card_id: CardId(spell.0), + targets: vec![], + payment_mode: CastPaymentMode::Auto, + }, + ) + .expect("Rousing Refrain must reach its final target prompt"); + + let WaitingFor::TargetSelection { + target_slots, + selection, + .. + } = &state.waiting_for + else { + panic!("Rousing Refrain must require a target opponent"); + }; + assert_eq!(target_slots.len(), 1, "the target prompt must be final"); + assert_eq!(selection.current_slot, 0, "the only target must be pending"); + let targets = selection.current_legal_targets.clone(); + assert_eq!( + targets, + vec![ + TargetRef::Player(PlayerId(0)), + TargetRef::Player(PlayerId(1)), + TargetRef::Player(PlayerId(2)), + ], + "reach guard: every opponent is a legal visible target" + ); + (state, targets) +} + +#[test] +fn decision_contract_filters_final_targets_that_cannot_complete_payment() { + let p3 = PlayerId(3); + let (state, targets) = rousing_refrain_final_target_state(false); + + for target in &targets { + let error = apply_as_current( + &mut state.clone(), + GameAction::ChooseTarget { + target: Some(target.clone()), + }, + ) + .expect_err("reach guard: final target selection must attempt the real payment"); + assert!( + error.to_string().contains("Cannot pay mana cost"), + "the unpayable final target must fail at payment, got {error}" + ); + } + + let contract = AiDecisionContract::issue(&state, p3); + assert_eq!( + contract + .candidates + .iter() + .map(|candidate| &candidate.action) + .collect::>(), + vec![&GameAction::CancelCast], + "the issued domain must contain only the reducer-completable cancellation" + ); + + let (payable, targets) = rousing_refrain_final_target_state(true); + let payable_contract = AiDecisionContract::issue(&payable, p3); + for target in targets { + let action = GameAction::ChooseTarget { + target: Some(target), + }; + assert!( + payable_contract.contains_action(&payable, &action), + "a final target remains issued when the same spell can pay its cost" + ); + } +} + +#[test] +fn decision_contract_keeps_reducer_completable_final_activation_targets() { + fn state_with_activation_mana(payable: bool) -> GameState { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let source = scenario + .add_creature(P0, "Targeting activation", 1, 1) + .with_ability_definition( + AbilityDefinition::new( + AbilityKind::Activated, + Effect::DealDamage { + amount: QuantityExpr::Fixed { value: 1 }, + target: TargetFilter::Any, + damage_source: None, + excess: None, + }, + ) + .cost(AbilityCost::Mana { + cost: ManaCost::generic(1), + }), + ) + .id(); + if payable { + scenario.with_mana_pool( + P0, + vec![ManaUnit::new( + ManaType::Colorless, + ObjectId(0), + false, + vec![], + )], + ); + } + let mut state = scenario.build().state().clone(); + apply_as_current( + &mut state, + GameAction::ActivateAbility { + source_id: source, + ability_index: 0, + }, + ) + .expect("activation must reach its final target prompt"); + state + } + + let action = GameAction::ChooseTarget { + target: Some(TargetRef::Player(P1)), + }; + let state = state_with_activation_mana(false); + let error = apply_as_current(&mut state.clone(), action.clone()) + .expect_err("reach guard: final activation target must attempt its unpaid mana cost"); + assert!( + error.to_string().contains("Cannot pay mana cost"), + "the unpayable final activation target must fail at payment, got {error}" + ); + let contract = AiDecisionContract::issue(&state, P0); + assert!( + !contract.contains_action(&state, &action), + "an unpayable final activation target must not be issued" + ); + + let state = state_with_activation_mana(true); + let contract = AiDecisionContract::issue(&state, P0); + assert!( + contract.contains_action(&state, &action), + "a reducer-completable final activation target must remain issued" + ); +} + /// Issue #7109: the decision contract must not offer an optional payment /// whose resulting deferred target set has no legal assignment. #[test] From 2bf1ae64429757d5ff026054b608e3d7d3859508 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 17:34:44 -0700 Subject: [PATCH 02/11] test: model nonfinal target selection fixtures --- crates/engine/src/ai_support/mod.rs | 25 +++++++++++++++++-------- crates/manabrew-compat/src/lib.rs | 25 +++++++++++++++++-------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/ai_support/mod.rs b/crates/engine/src/ai_support/mod.rs index 3de9b0076d..3957eec1a8 100644 --- a/crates/engine/src/ai_support/mod.rs +++ b/crates/engine/src/ai_support/mod.rs @@ -6171,7 +6171,7 @@ mod tests { } #[test] - fn target_selection_legal_actions_use_current_targets_without_simulation() { + fn earlier_target_selection_legal_actions_use_current_targets_without_simulation() { let mut state = setup_priority(); let targets: Vec = (0..25) .map(|i| { @@ -6198,13 +6198,22 @@ mod tests { state.waiting_for = WaitingFor::TargetSelection { player: PlayerId(0), pending_cast, - target_slots: vec![crate::types::game_state::TargetSelectionSlot { - legal_targets: targets.clone(), - optional: true, - chooser: None, - effect_kind: EffectKind::NoOp, - effect_detail: TargetEffectDetail::None, - }], + target_slots: vec![ + crate::types::game_state::TargetSelectionSlot { + legal_targets: targets.clone(), + optional: true, + chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, + }, + crate::types::game_state::TargetSelectionSlot { + legal_targets: Vec::new(), + optional: true, + chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, + }, + ], mode_labels: Vec::new(), selection: crate::types::game_state::TargetSelectionProgress { current_slot: 0, diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index b2ddfb2d2e..1085eea3cf 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -5255,7 +5255,7 @@ mod tests { assert_eq!(json["input"]["presentation"]["title"], "Choose target"); } - /// Build a `TargetSelection` board-target prompt whose single slot carries + /// Build an earlier `TargetSelection` board-target prompt whose active slot carries /// `effect_kind`, driving the real engine projection /// (`derive_viewer_interaction` -> `target_intent`) and the real adapter /// mapping. Returns the serialized prompt. @@ -5280,13 +5280,22 @@ mod tests { state.waiting_for = WaitingFor::TargetSelection { player: PlayerId(0), pending_cast: dummy_pending_cast(), - target_slots: vec![TargetSelectionSlot { - legal_targets: legal.clone(), - optional: false, - chooser: None, - effect_kind, - effect_detail, - }], + target_slots: vec![ + TargetSelectionSlot { + legal_targets: legal.clone(), + optional: false, + chooser: None, + effect_kind, + effect_detail, + }, + TargetSelectionSlot { + legal_targets: Vec::new(), + optional: true, + chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, + }, + ], mode_labels: Vec::new(), selection: TargetSelectionProgress { current_slot: 0, From 79d5fd09172782cae962c4134c5849777b362f91 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 17:50:44 -0700 Subject: [PATCH 03/11] test: keep stale-target fixture nonfinal --- crates/engine/src/ai_support/mod.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/ai_support/mod.rs b/crates/engine/src/ai_support/mod.rs index 3957eec1a8..d5e511af6f 100644 --- a/crates/engine/src/ai_support/mod.rs +++ b/crates/engine/src/ai_support/mod.rs @@ -6610,13 +6610,22 @@ mod tests { state.waiting_for = WaitingFor::TargetSelection { player: PlayerId(0), pending_cast, - target_slots: vec![crate::types::game_state::TargetSelectionSlot { - legal_targets: vec![target.clone()], - optional: true, - chooser: None, - effect_kind: EffectKind::NoOp, - effect_detail: TargetEffectDetail::None, - }], + target_slots: vec![ + crate::types::game_state::TargetSelectionSlot { + legal_targets: vec![target.clone()], + optional: true, + chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, + }, + crate::types::game_state::TargetSelectionSlot { + legal_targets: Vec::new(), + optional: true, + chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, + }, + ], mode_labels: Vec::new(), selection: crate::types::game_state::TargetSelectionProgress { current_slot: 0, From 272dbba2499eb49a2166d24f3ab1f5e263a83e6d Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 17:59:39 -0700 Subject: [PATCH 04/11] fix: validate all AI target selections --- crates/engine/src/ai_support/context.rs | 20 ++-- crates/engine/src/ai_support/mod.rs | 15 +-- .../tests/integration/ai_decision_contract.rs | 100 +++++++++++++++++- crates/manabrew-compat/src/lib.rs | 2 +- 4 files changed, 105 insertions(+), 32 deletions(-) diff --git a/crates/engine/src/ai_support/context.rs b/crates/engine/src/ai_support/context.rs index 6656544b4d..abfe280528 100644 --- a/crates/engine/src/ai_support/context.rs +++ b/crates/engine/src/ai_support/context.rs @@ -103,20 +103,12 @@ impl AiDecisionContract { } pub(crate) fn target_selection_requires_reducer_validation(state: &GameState) -> bool { - let WaitingFor::TargetSelection { - target_slots, - selection, - .. - } = &state.waiting_for - else { - return false; - }; - - // CR 601.2c + CR 601.2e-h + CR 602.2b: completing the final target - // selection can immediately check legality and pay the proposed spell or - // activation cost. Earlier selections cannot complete that transition, but - // every final candidate must be reducer-validated before the AI receives it. - selection.current_slot.checked_add(1) == Some(target_slots.len()) + // CR 601.2c + CR 601.2e-h + CR 602.2b: selecting a target can complete + // target declaration and immediately check legality and pay the proposed + // spell or activation cost. A later optional slot can become auto-skippable + // only after this target is chosen, so the reducer is the sole authority for + // whether a particular candidate completes the transition. + matches!(&state.waiting_for, WaitingFor::TargetSelection { .. }) } /// Whether a decision can alter the target requirements of an in-progress cast. diff --git a/crates/engine/src/ai_support/mod.rs b/crates/engine/src/ai_support/mod.rs index d5e511af6f..0616bb6343 100644 --- a/crates/engine/src/ai_support/mod.rs +++ b/crates/engine/src/ai_support/mod.rs @@ -6171,7 +6171,7 @@ mod tests { } #[test] - fn earlier_target_selection_legal_actions_use_current_targets_without_simulation() { + fn target_selection_legal_actions_use_current_targets_with_reducer_validation() { let mut state = setup_priority(); let targets: Vec = (0..25) .map(|i| { @@ -6207,7 +6207,7 @@ mod tests { effect_detail: TargetEffectDetail::None, }, crate::types::game_state::TargetSelectionSlot { - legal_targets: Vec::new(), + legal_targets: vec![targets[0].clone()], optional: true, chooser: None, effect_kind: EffectKind::NoOp, @@ -6222,12 +6222,8 @@ mod tests { }, }; - crate::game::perf_counters::reset(); let (actions, spell_costs, grouped) = legal_actions_full(&state); - let counters = crate::game::perf_counters::snapshot(); - assert_eq!(counters.state_clone_for_legality, 0); - assert_eq!(counters.priority_cast_probe_builds, 0); assert_eq!( actions .iter() @@ -6619,7 +6615,7 @@ mod tests { effect_detail: TargetEffectDetail::None, }, crate::types::game_state::TargetSelectionSlot { - legal_targets: Vec::new(), + legal_targets: vec![target.clone()], optional: true, chooser: None, effect_kind: EffectKind::NoOp, @@ -6634,13 +6630,8 @@ mod tests { }, }; - crate::game::perf_counters::reset(); let (actions, _spell_costs, _grouped) = legal_actions_full(&state); - assert_eq!( - crate::game::perf_counters::snapshot().state_clone_for_legality, - 0 - ); assert_eq!(actions, vec![GameAction::ChooseTarget { target: None }]); } diff --git a/crates/engine/tests/integration/ai_decision_contract.rs b/crates/engine/tests/integration/ai_decision_contract.rs index 0ec6d9ce00..b63b103545 100644 --- a/crates/engine/tests/integration/ai_decision_contract.rs +++ b/crates/engine/tests/integration/ai_decision_contract.rs @@ -4,8 +4,8 @@ use engine::game::scenario::{GameScenario, P0, P1}; use engine::game::zones::create_object; use engine::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AdditionalCost, - AdditionalCostRepeatability, Effect, QuantityExpr, TargetFilter, TargetRef, TypeFilter, - TypedFilter, + AdditionalCostRepeatability, Effect, MultiTargetSpec, QuantityExpr, TargetFilter, TargetRef, + TypeFilter, TypedFilter, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; @@ -19,7 +19,10 @@ use std::sync::Arc; const ROUSING_REFRAIN_ORACLE: &str = "Add {R} for each card in target opponent's hand. Until end of turn, you don't lose this mana as steps and phases end. Exile Rousing Refrain with three time counters on it."; -fn rousing_refrain_final_target_state(payable: bool) -> (GameState, Vec) { +fn rousing_refrain_target_state( + payable: bool, + target_max: Option, +) -> (GameState, Vec) { let p3 = PlayerId(3); let mut scenario = GameScenario::new_n_player(4, 7114); scenario.at_phase(Phase::PreCombatMain); @@ -43,6 +46,18 @@ fn rousing_refrain_final_target_state(payable: bool) -> (GameState, Vec (GameState, Vec (GameState, Vec (GameState, Vec) { + let (state, targets) = rousing_refrain_target_state(payable, None); + let WaitingFor::TargetSelection { target_slots, .. } = &state.waiting_for else { + panic!("Rousing Refrain must remain in target selection"); + }; + assert_eq!(target_slots.len(), 1, "the target prompt must be final"); + (state, targets) +} + #[test] fn decision_contract_filters_final_targets_that_cannot_complete_payment() { let p3 = PlayerId(3); @@ -123,6 +149,70 @@ fn decision_contract_filters_final_targets_that_cannot_complete_payment() { } } +#[test] +fn decision_contract_validates_a_target_before_a_dynamically_empty_optional_tail() { + let p3 = PlayerId(3); + let (mut state, _) = rousing_refrain_target_state(false, Some(4)); + + for player in [PlayerId(0), PlayerId(1)] { + apply_as_current( + &mut state, + GameAction::ChooseTarget { + target: Some(TargetRef::Player(player)), + }, + ) + .expect("the first two distinct opponent targets must be accepted"); + } + + let WaitingFor::TargetSelection { + target_slots, + selection, + .. + } = &state.waiting_for + else { + panic!("Rousing Refrain must remain in target selection"); + }; + assert_eq!( + selection.current_slot, 2, + "the third target must be pending" + ); + assert_eq!( + target_slots[3].legal_targets, + vec![ + TargetRef::Player(PlayerId(0)), + TargetRef::Player(PlayerId(1)), + TargetRef::Player(PlayerId(2)), + ], + "the trailing slot starts with targets and only becomes empty after prior choices" + ); + assert_eq!( + selection.current_legal_targets, + vec![TargetRef::Player(PlayerId(2))], + "the final remaining target must be selected before the exhausted tail auto-skips" + ); + + let action = GameAction::ChooseTarget { + target: Some(TargetRef::Player(PlayerId(2))), + }; + let error = apply_as_current(&mut state.clone(), action.clone()) + .expect_err("the dynamically empty tail is auto-skipped before payment"); + assert!( + error.to_string().contains("Cannot pay mana cost"), + "the auto-skipped tail must still reach payment, got {error}" + ); + + let contract = AiDecisionContract::issue(&state, p3); + assert_eq!( + contract + .candidates + .iter() + .map(|candidate| &candidate.action) + .collect::>(), + vec![&GameAction::CancelCast], + "a target that completes through a dynamically auto-skipped tail must not be issued" + ); +} + #[test] fn decision_contract_keeps_reducer_completable_final_activation_targets() { fn state_with_activation_mana(payable: bool) -> GameState { diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index 1085eea3cf..117389d438 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -5289,7 +5289,7 @@ mod tests { effect_detail, }, TargetSelectionSlot { - legal_targets: Vec::new(), + legal_targets: legal.clone(), optional: true, chooser: None, effect_kind: EffectKind::NoOp, From e5211180212d0ea3266b4abbef924b46d9680ecc Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 18:07:07 -0700 Subject: [PATCH 05/11] fix: remove unused target-slot binding --- crates/engine/tests/integration/ai_decision_contract.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tests/integration/ai_decision_contract.rs b/crates/engine/tests/integration/ai_decision_contract.rs index b63b103545..0a83c62bc6 100644 --- a/crates/engine/tests/integration/ai_decision_contract.rs +++ b/crates/engine/tests/integration/ai_decision_contract.rs @@ -73,7 +73,7 @@ fn rousing_refrain_target_state( .expect("Rousing Refrain must reach its final target prompt"); let WaitingFor::TargetSelection { - target_slots, + target_slots: _, selection, .. } = &state.waiting_for From c392f8a6320eb8de104337aaaaceb0a8792ae23c Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 18:23:48 -0700 Subject: [PATCH 06/11] test: cover dynamic target tail validation --- crates/engine/src/ai_support/mod.rs | 9 ++- .../tests/integration/ai_decision_contract.rs | 68 ++++++++----------- 2 files changed, 35 insertions(+), 42 deletions(-) diff --git a/crates/engine/src/ai_support/mod.rs b/crates/engine/src/ai_support/mod.rs index 0616bb6343..d0d5ef8503 100644 --- a/crates/engine/src/ai_support/mod.rs +++ b/crates/engine/src/ai_support/mod.rs @@ -6632,7 +6632,14 @@ mod tests { let (actions, _spell_costs, _grouped) = legal_actions_full(&state); - assert_eq!(actions, vec![GameAction::ChooseTarget { target: None }]); + assert!(actions.contains(&GameAction::ChooseTarget { target: None })); + assert!(actions.contains(&GameAction::CancelCast)); + assert!( + !actions + .iter() + .any(|action| matches!(action, GameAction::ChooseTarget { target: Some(_) })), + "stale slot targets must not be reissued" + ); } /// False-positive sweep (CR 103.5 / TL:R 906.6a): the simultaneous diff --git a/crates/engine/tests/integration/ai_decision_contract.rs b/crates/engine/tests/integration/ai_decision_contract.rs index 0a83c62bc6..ffbb04c351 100644 --- a/crates/engine/tests/integration/ai_decision_contract.rs +++ b/crates/engine/tests/integration/ai_decision_contract.rs @@ -4,12 +4,14 @@ use engine::game::scenario::{GameScenario, P0, P1}; use engine::game::zones::create_object; use engine::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AdditionalCost, - AdditionalCostRepeatability, Effect, MultiTargetSpec, QuantityExpr, TargetFilter, TargetRef, - TypeFilter, TypedFilter, + AdditionalCostRepeatability, Effect, EffectKind, QuantityExpr, TargetEffectDetail, + TargetFilter, TargetRef, TypeFilter, TypedFilter, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; -use engine::types::game_state::{CastPaymentMode, GameState, WaitingFor}; +use engine::types::game_state::{ + CastPaymentMode, GameState, TargetSelectionConstraint, TargetSelectionSlot, WaitingFor, +}; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; use engine::types::phase::Phase; @@ -19,10 +21,7 @@ use std::sync::Arc; const ROUSING_REFRAIN_ORACLE: &str = "Add {R} for each card in target opponent's hand. Until end of turn, you don't lose this mana as steps and phases end. Exile Rousing Refrain with three time counters on it."; -fn rousing_refrain_target_state( - payable: bool, - target_max: Option, -) -> (GameState, Vec) { +fn rousing_refrain_target_state(payable: bool) -> (GameState, Vec) { let p3 = PlayerId(3); let mut scenario = GameScenario::new_n_player(4, 7114); scenario.at_phase(Phase::PreCombatMain); @@ -46,18 +45,6 @@ fn rousing_refrain_target_state( } let mut state = scenario.build().state().clone(); - if let Some(target_max) = target_max { - Arc::make_mut( - &mut state - .objects - .get_mut(&spell) - .expect("Rousing Refrain must exist") - .abilities, - )[0] - .multi_target = Some(MultiTargetSpec::up_to(QuantityExpr::Fixed { - value: target_max as i32, - })); - } state.active_player = p3; state.priority_player = p3; state.waiting_for = WaitingFor::Priority { player: p3 }; @@ -98,7 +85,7 @@ fn rousing_refrain_target_state( } fn rousing_refrain_final_target_state(payable: bool) -> (GameState, Vec) { - let (state, targets) = rousing_refrain_target_state(payable, None); + let (state, targets) = rousing_refrain_target_state(payable); let WaitingFor::TargetSelection { target_slots, .. } = &state.waiting_for else { panic!("Rousing Refrain must remain in target selection"); }; @@ -152,20 +139,11 @@ fn decision_contract_filters_final_targets_that_cannot_complete_payment() { #[test] fn decision_contract_validates_a_target_before_a_dynamically_empty_optional_tail() { let p3 = PlayerId(3); - let (mut state, _) = rousing_refrain_target_state(false, Some(4)); - - for player in [PlayerId(0), PlayerId(1)] { - apply_as_current( - &mut state, - GameAction::ChooseTarget { - target: Some(TargetRef::Player(player)), - }, - ) - .expect("the first two distinct opponent targets must be accepted"); - } + let (mut state, _) = rousing_refrain_target_state(false); let WaitingFor::TargetSelection { target_slots, + pending_cast, selection, .. } = &state.waiting_for @@ -173,26 +151,34 @@ fn decision_contract_validates_a_target_before_a_dynamically_empty_optional_tail panic!("Rousing Refrain must remain in target selection"); }; assert_eq!( - selection.current_slot, 2, - "the third target must be pending" + selection.current_slot, 0, + "the first target must be pending" ); + pending_cast.target_constraints = vec![TargetSelectionConstraint::DifferentTargetPlayers]; + target_slots.push(TargetSelectionSlot { + legal_targets: vec![TargetRef::Player(PlayerId(0))], + optional: true, + chooser: None, + effect_kind: EffectKind::NoOp, + effect_detail: TargetEffectDetail::None, + }); assert_eq!( - target_slots[3].legal_targets, + target_slots[1].legal_targets, + vec![TargetRef::Player(PlayerId(0))], + "the trailing slot starts with targets and only becomes empty after prior choices" + ); + assert_eq!( + selection.current_legal_targets, vec![ TargetRef::Player(PlayerId(0)), TargetRef::Player(PlayerId(1)), TargetRef::Player(PlayerId(2)), ], - "the trailing slot starts with targets and only becomes empty after prior choices" - ); - assert_eq!( - selection.current_legal_targets, - vec![TargetRef::Player(PlayerId(2))], - "the final remaining target must be selected before the exhausted tail auto-skips" + "the first target remains independently legal" ); let action = GameAction::ChooseTarget { - target: Some(TargetRef::Player(PlayerId(2))), + target: Some(TargetRef::Player(PlayerId(0))), }; let error = apply_as_current(&mut state.clone(), action.clone()) .expect_err("the dynamically empty tail is auto-skipped before payment"); From 60d6ec7299a6546438d97224519648cd0b84ce8d Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 18:27:11 -0700 Subject: [PATCH 07/11] test: give dynamic target tail a real sink --- .../tests/integration/ai_decision_contract.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/engine/tests/integration/ai_decision_contract.rs b/crates/engine/tests/integration/ai_decision_contract.rs index ffbb04c351..8998654461 100644 --- a/crates/engine/tests/integration/ai_decision_contract.rs +++ b/crates/engine/tests/integration/ai_decision_contract.rs @@ -4,8 +4,8 @@ use engine::game::scenario::{GameScenario, P0, P1}; use engine::game::zones::create_object; use engine::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AdditionalCost, - AdditionalCostRepeatability, Effect, EffectKind, QuantityExpr, TargetEffectDetail, - TargetFilter, TargetRef, TypeFilter, TypedFilter, + AdditionalCostRepeatability, Effect, EffectKind, QuantityExpr, ResolvedAbility, + TargetEffectDetail, TargetFilter, TargetRef, TypeFilter, TypedFilter, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; @@ -155,6 +155,16 @@ fn decision_contract_validates_a_target_before_a_dynamically_empty_optional_tail "the first target must be pending" ); pending_cast.target_constraints = vec![TargetSelectionConstraint::DifferentTargetPlayers]; + let mut trailing_ability = ResolvedAbility::new( + Effect::TargetOnly { + target: TargetFilter::SpecificPlayer { id: PlayerId(0) }, + }, + Vec::new(), + pending_cast.ability.source_id, + p3, + ); + trailing_ability.optional_targeting = true; + pending_cast.ability.sub_ability = Some(Box::new(trailing_ability)); target_slots.push(TargetSelectionSlot { legal_targets: vec![TargetRef::Player(PlayerId(0))], optional: true, From ac447d868c0784ad9f4ac59e9d8eb6ef954fd917 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 18:31:29 -0700 Subject: [PATCH 08/11] fix: import target detail from game state --- crates/engine/tests/integration/ai_decision_contract.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/engine/tests/integration/ai_decision_contract.rs b/crates/engine/tests/integration/ai_decision_contract.rs index 8998654461..ed02db410a 100644 --- a/crates/engine/tests/integration/ai_decision_contract.rs +++ b/crates/engine/tests/integration/ai_decision_contract.rs @@ -4,13 +4,14 @@ use engine::game::scenario::{GameScenario, P0, P1}; use engine::game::zones::create_object; use engine::types::ability::{ AbilityCondition, AbilityCost, AbilityDefinition, AbilityKind, AdditionalCost, - AdditionalCostRepeatability, Effect, EffectKind, QuantityExpr, ResolvedAbility, - TargetEffectDetail, TargetFilter, TargetRef, TypeFilter, TypedFilter, + AdditionalCostRepeatability, Effect, EffectKind, QuantityExpr, ResolvedAbility, TargetFilter, + TargetRef, TypeFilter, TypedFilter, }; use engine::types::actions::GameAction; use engine::types::card_type::CoreType; use engine::types::game_state::{ - CastPaymentMode, GameState, TargetSelectionConstraint, TargetSelectionSlot, WaitingFor, + CastPaymentMode, GameState, TargetEffectDetail, TargetSelectionConstraint, TargetSelectionSlot, + WaitingFor, }; use engine::types::identifiers::{CardId, ObjectId}; use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; From 480257ce9a556e5e48310066f83a4031b345bd53 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 18:40:18 -0700 Subject: [PATCH 09/11] test: mutate target selection fixture --- crates/engine/tests/integration/ai_decision_contract.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/tests/integration/ai_decision_contract.rs b/crates/engine/tests/integration/ai_decision_contract.rs index ed02db410a..175b85676c 100644 --- a/crates/engine/tests/integration/ai_decision_contract.rs +++ b/crates/engine/tests/integration/ai_decision_contract.rs @@ -147,7 +147,7 @@ fn decision_contract_validates_a_target_before_a_dynamically_empty_optional_tail pending_cast, selection, .. - } = &state.waiting_for + } = &mut state.waiting_for else { panic!("Rousing Refrain must remain in target selection"); }; From bc3c413e05fe8ee81619c5e7279d6accc7fce15b Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 18:54:56 -0700 Subject: [PATCH 10/11] test: expect reducer validation for AI targets --- crates/phase-ai/src/search.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/phase-ai/src/search.rs b/crates/phase-ai/src/search.rs index ed03e921fc..19df3651a9 100644 --- a/crates/phase-ai/src/search.rs +++ b/crates/phase-ai/src/search.rs @@ -8847,7 +8847,7 @@ mod tests { } #[test] - fn unmodeled_target_selection_uses_an_issued_forward_action_without_scoring() { + fn unmodeled_target_selection_uses_a_reducer_validated_forward_action() { let mut state = spell_target_selection_state( vec![ TargetRef::Player(PlayerId(0)), @@ -8875,9 +8875,9 @@ mod tests { let contract = AiDecisionContract::issue(&state, PlayerId(0)); assert_eq!( - counters.state_clone_for_legality, 0, - "an unsupported spell's engine-issued target slot must not replay cast/payment \ - simulation before the AI can answer" + counters.state_clone_for_legality, 3, + "every target choice and CancelCast must pass through the reducer before the \ + AI receives the engine-issued decision domain" ); assert!( action.is_some(), From 2891e45c5d37b0ab424f1cd2ab71eb2eaf2cd8a8 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Tue, 11 Aug 2026 19:02:54 -0700 Subject: [PATCH 11/11] test: cover validated AI target actions --- crates/engine/src/ai_support/mod.rs | 3 ++- .../engine/tests/integration/ai_decision_contract.rs | 12 ++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/ai_support/mod.rs b/crates/engine/src/ai_support/mod.rs index d0d5ef8503..ac1be947b4 100644 --- a/crates/engine/src/ai_support/mod.rs +++ b/crates/engine/src/ai_support/mod.rs @@ -6232,7 +6232,8 @@ mod tests { 25 ); assert!(actions.contains(&GameAction::ChooseTarget { target: None })); - assert_eq!(actions.len(), 26); + assert_eq!(actions.len(), 27); + assert!(actions.contains(&GameAction::CancelCast)); assert!(spell_costs.is_empty()); assert!(grouped.is_empty()); assert!(actions diff --git a/crates/engine/tests/integration/ai_decision_contract.rs b/crates/engine/tests/integration/ai_decision_contract.rs index 175b85676c..f74fa21015 100644 --- a/crates/engine/tests/integration/ai_decision_contract.rs +++ b/crates/engine/tests/integration/ai_decision_contract.rs @@ -205,8 +205,16 @@ fn decision_contract_validates_a_target_before_a_dynamically_empty_optional_tail .iter() .map(|candidate| &candidate.action) .collect::>(), - vec![&GameAction::CancelCast], - "a target that completes through a dynamically auto-skipped tail must not be issued" + vec![ + &GameAction::ChooseTarget { + target: Some(TargetRef::Player(PlayerId(1))), + }, + &GameAction::ChooseTarget { + target: Some(TargetRef::Player(PlayerId(2))), + }, + &GameAction::CancelCast, + ], + "only the target that completes through the dynamically auto-skipped tail must be excluded" ); }