From 1fc4211ef88513f4c6514fc57723d945eb076bad Mon Sep 17 00:00:00 2001 From: matthewevans Date: Mon, 27 Jul 2026 11:30:12 -0700 Subject: [PATCH 1/4] feat(manabrew-compat): serve unmapped prompts from the interaction projection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The adapter named 42 of the engine's 127 `WaitingFor` variants; the other 85 fell through to `local.prompt-unsupported`. Closing them one at a time would have meant 85 hand-written mappings, each wiring three sites and re-deriving min/max and constraint logic the engine already computes — duplicated game logic inside a serialization boundary. The engine had already done the classification. `human_response_model` (`game/interaction.rs`) matches all 127 variants with **no catch-all arm**, so coverage is compiler-enforced rather than sampled, and collapses them onto 20 response models. `opportunity_for_slot` then renders each as one of the 12 `InteractionResponseSpec` variants — the whole enum, so the projector is total over it. Reading it end to end also settles what looked like it needed runtime observation: almost every spec field is a source constant (`confirm` is literally `Explicit` at every site but `Select`), and what varies is numeric bounds and candidate lists, which an adapter passes through rather than reproduces. So the wildcard arm now routes to the projection instead of refusing. Decisions the engine renders as a finite `ExactChoices` list map generically onto `ChooseFromSelection` — a pre-materialized candidate list is exactly that family's shape, so the mapping is total and needs no per-variant judgement. Measured over the production half of the file: **52 of the 85 map this way**, taking prompt coverage from 42/127 to 94/127. The remaining 33 are schema-valued (19 `Select`, 6 `TargetSequence`, 3 `AmountAssignments`, and six singletons); their response space is unbounded, no single family carries their bounds, and they still fail closed under a declared code rather than being flattened into a selection that would lose them. What makes this work without the adapter judging mechanics is that `InteractionChoice.surfaces` carries payload shape as *data* — `Object { name, zone, controller, power, tapped }`, `Player { seat }`, `Value { .. }`. `choice_label` joins every naming surface rather than taking the first, because choices can share an object and differ only in a `Value`: the priority projection offers auto- and manual-payment casts of one spell that way, and a first-surface label would render them identically. Engine side, `resolve_interaction_response` is a non-mutating sibling of `submit_interaction`, which now delegates to it — so the two cannot drift. The adapter needs the materialization without the application, because it hands a `GameAction` back to its caller. It must not re-derive that mapping: `materialize_response` is exhaustive over `HumanResponseModel`, so an external reimplementation would keep compiling while silently going stale. Authorization is unchanged — `slot_for_submission` still authenticates the actor, so dropping the mutation does not turn this into a way to read another seat's decision. Two failure modes are deliberately closed rather than papered over: * A viewer can be the authorized submitter for more than one interaction slot — a decision both seats owe at once. The wire carries one prompt, so serving `.first()` would answer one seat and silently drop the other. Both the prompt and the response path require exactly one opportunity. * The response gate cannot be a `WaitingFor` list for this family, and must not be the projection either: `WaitingFor::Priority` also projects a finite list, so gating on that alone would accept a `chooseFromSelection` answer to a `chooseAction` prompt — a straight breach of the documented rule that a response is valid only if its family matches the open prompt. It instead rebuilds the prompt and checks the family, so it cannot drift from the builder because it *is* the builder. `PreparedManabrewSnapshot` gains the projection because it is derivable only from raw state, which `build_prompt_input` does not have — `derive_viewer_- interaction` reads authorization from the authoritative state and every presentation surface from the filtered one. Verified by running, not by inference: 77/77 `manabrew-compat` tests pass and `cargo clippy -p manabrew-compat` exits 0, both in an isolated `CARGO_TARGET_DIR` because **no Tilt resource runs this crate's tests** — `test-engine` is `-p engine` and `test-ai` is `-p phase-ai`, so a green board would have been silence, not evidence. The engine half is Tilt-green with freshness confirmed against file mtimes. The new test uses `TopOrBottomChoice` because it is genuinely unmapped and its choices differ only by a `Value` surface, so it also pins the label logic. Its non-vacuity guard is the same waiting state with no bound interaction authority, which must still be unsupported — otherwise the first test could be passing for an unrelated reason. --- crates/engine/src/game/interaction.rs | 49 ++- .../tests/integration/interaction_contract.rs | 32 +- crates/manabrew-compat/src/lib.rs | 335 +++++++++++++++++- 3 files changed, 392 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/game/interaction.rs b/crates/engine/src/game/interaction.rs index 823b4ea1c9..53cc15c07f 100644 --- a/crates/engine/src/game/interaction.rs +++ b/crates/engine/src/game/interaction.rs @@ -8869,6 +8869,41 @@ pub fn preview_interaction( } } +/// Materialize the `GameAction` an interaction response denotes, **without** +/// applying it. +/// +/// [`submit_interaction`] is the mutating path and delegates here for everything +/// up to the reducer, so the two cannot drift. This variant exists for consumers +/// that own their own dispatch and need the action itself — the ManaBrew adapter +/// translates a client's prompt answer into a `GameAction` and returns it to its +/// caller rather than applying it. +/// +/// Such a consumer must never re-derive this mapping. `materialize_response` +/// matches exhaustively on `HumanResponseModel` with no catch-all arm, so the +/// compiler forces every new decision family through it; a reimplementation +/// living outside the engine would keep compiling while silently going stale. +/// +/// Dropping the mutation does not weaken authorization. `slot_for_submission` +/// still authenticates `actor` against the slot, so this cannot be used to +/// materialize a decision that belongs to another player. +pub fn resolve_interaction_response( + state: &GameState, + actor: PlayerId, + submission: &InteractionSubmission, +) -> Result { + bound_string(submission.interaction_id.as_str())?; + validate_response_bounds(&submission.response)?; + slot_for_submission(state, actor, &submission.interaction_id)?; + let filtered = visibility::filter_state_for_viewer(state, actor); + let (action, _) = materialize_response( + state, + &filtered, + &submission.interaction_id, + &submission.response, + )?; + Ok(action) +} + /// Hidden engine-only submission entry point. The opaque interaction and choice /// IDs are looked up against current trusted state, authorization is rechecked, /// projection is recomputed from a viewer-filtered clone, and the materialized @@ -8878,17 +8913,13 @@ pub fn submit_interaction( actor: PlayerId, submission: InteractionSubmission, ) -> Result { - bound_string(submission.interaction_id.as_str())?; - validate_response_bounds(&submission.response)?; + let action = resolve_interaction_response(state, actor, &submission)?; + // Re-read the slot rather than threading it out of `resolve_*`: keeping that + // function's return to the action alone is what makes it usable as a public + // seam. The lookup is a scan of `active_interaction_slots`, which holds one + // slot per pending decision, and it has already succeeded once here. let semantic_owner = PlayerId(slot_for_submission(state, actor, &submission.interaction_id)?.semantic_owner); - let filtered = visibility::filter_state_for_viewer(state, actor); - let (action, _) = materialize_response( - state, - &filtered, - &submission.interaction_id, - &submission.response, - )?; apply_interaction(state, actor, semantic_owner, action).map_err(|_error: EngineError| { InteractionSubmitError { code: InteractionReasonCode::ReducerRejected, diff --git a/crates/engine/tests/integration/interaction_contract.rs b/crates/engine/tests/integration/interaction_contract.rs index 03119b4496..2ef04b7dbb 100644 --- a/crates/engine/tests/integration/interaction_contract.rs +++ b/crates/engine/tests/integration/interaction_contract.rs @@ -5,7 +5,8 @@ use engine::analysis::decision_template::{ }; use engine::game::engine::apply; use engine::game::interaction::{ - bind_interaction_authority, derive_viewer_interaction, preview_interaction, submit_interaction, + bind_interaction_authority, derive_viewer_interaction, preview_interaction, + resolve_interaction_response, submit_interaction, }; use engine::game::scenario::{GameScenario, P0, P1}; use engine::game::scenario_db::GameScenarioDbExt; @@ -291,6 +292,35 @@ fn bottom_card_opportunities_use_and_only_materialize_select_responses() { ); } +#[test] +fn resolving_a_response_materializes_the_advertised_action_under_the_same_authorization() { + let mut state = GameState::new_two_player(42); + bind(&mut state, "resolve-seam"); + let witness = progress_witness(&state, P0); + + // Authorization parity with `submit_interaction` is the entire risk of a + // non-mutating sibling: without the actor check it would become a way to + // materialize — and therefore to read — a decision belonging to another + // seat. Nothing here asserts that the state is unchanged, because + // `resolve_interaction_response` takes `&GameState`: non-mutation is a + // borrow-checker guarantee, and a test of it would pass for reasons that + // have nothing to do with this function. + let unauthorized = resolve_interaction_response(&state, P1, &witness) + .expect_err("resolving authorizes against the actor, not merely the interaction id"); + assert_eq!(unauthorized.code, InteractionReasonCode::NotAuthorized); + + let action = resolve_interaction_response(&state, P0, &witness) + .expect("the advertised progress witness resolves to the action it denotes"); + assert_eq!(action, GameAction::PassPriority); + + // The same witness really is submittable, so the resolution above concerns a + // live decision rather than one the engine would have refused anyway. + // Equivalence between the two paths needs no assertion: `submit_interaction` + // delegates here, so they cannot disagree. + submit_interaction(&mut state, P0, witness) + .expect("the witness the projection advertised is submittable"); +} + #[test] fn priority_projection_previews_submits_and_rejects_stale_or_unauthorized_ids() { let mut state = GameState::new_two_player(42); diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index 6a21c13767..aff2aee3f0 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -18,6 +18,7 @@ use engine::game::derived::derive_display_state; use engine::game::derived_views::{derive_views, DerivedViews}; use engine::game::filter_state_for_viewer; use engine::game::game_object::{AttachTarget, GameObject}; +use engine::game::interaction::{derive_viewer_interaction, resolve_interaction_response}; use engine::game::turn_control; use engine::types::ability::TargetRef; use engine::types::card::CardFace; @@ -25,6 +26,10 @@ use engine::types::game_state::{ GameState, ManaChoice, ManaChoicePrompt, MulliganDecisionPhase, PendingMulliganAction, ShardChoice, StackEntryKind, WaitingFor, }; +use engine::types::interaction::{ + InteractionChoice, InteractionOpportunityResponse, InteractionPresentationSurface, + InteractionResponse, InteractionSubmission, ViewerInteraction, +}; use engine::types::mana::{ManaColor as EngineManaColor, ManaCost, ManaCostShard, ManaType}; use engine::types::phase::Phase; use engine::types::player::{PlayerCounterKind, PlayerId}; @@ -134,6 +139,20 @@ pub struct PreparedManabrewSnapshot { /// object here is what lets `build_prompt` construct the `CardDto` later, /// where a `CardTextLookup` is finally in scope. pub source_card_object: Option, + /// The engine's own projection of what this viewer may answer right now. + /// + /// Captured here because it is derivable only from **raw** state, which + /// `build_prompt_input` no longer has: `derive_viewer_interaction` reads + /// authorization and capability identity from the authoritative state and + /// every presentation surface from the filtered one, and collapsing that to + /// a single filtered state would silently change what the viewer is told. + /// + /// Derived unconditionally rather than on demand. One projection per prompt + /// is proportionate — a prompt is a human decision point, not a search-tree + /// node — and making it conditional would mean deciding *here* which waiting + /// states the generic path serves, which is precisely the per-variant + /// bookkeeping this projection exists to remove. + pub interaction: ViewerInteraction, } impl PreparedManabrewSnapshot { @@ -194,6 +213,9 @@ pub fn prepare_snapshot_with_prompt_id( .and_then(|id| raw_state.objects.get(&id)) .cloned(); let mut state = filter_state_for_viewer(raw_state, viewer); + // Projected from the plain viewer filter, before `derive_display_state`, so + // the adapter sees exactly what every other interaction consumer sees. + let interaction = derive_viewer_interaction(raw_state, &state, viewer); derive_display_state(&mut state); let derived = derive_views(&state, Some(viewer)); @@ -207,6 +229,7 @@ pub fn prepare_snapshot_with_prompt_id( spell_costs, legal_actions_by_object, source_card_object, + interaction, }) } @@ -1335,7 +1358,7 @@ pub fn unsupported_protocol_capabilities() -> &'static [UnsupportedCapability] { /// /// `upstream.` = the protocol has no primitive for something the engine can do. /// `local.` = the protocol has the primitive but this engine cannot source it. -static UNSUPPORTED_PROTOCOL_CAPABILITIES: [UnsupportedCapability; 79] = [ +static UNSUPPORTED_PROTOCOL_CAPABILITIES: [UnsupportedCapability; 81] = [ UnsupportedCapability { code: "upstream.object-selection-missing", area: "prompts", @@ -1526,8 +1549,20 @@ static UNSUPPORTED_PROTOCOL_CAPABILITIES: [UnsupportedCapability; 79] = [ UnsupportedCapability { code: "local.prompt-unsupported", area: "prompts", - reason: "Catch-all for the wildcard arm of build_prompt_input(). Phase has 127 WaitingFor variants and this adapter names 34 of them, so any of the remaining 93 that becomes current produces this code instead of a prompt. It is a coverage statement about the adapter's match, not a claim about any mechanic: WaitingFor::PhyrexianPayment and WaitingFor::RevealChoice both land here today despite being fully modeled at both ends. A shape census (bucket each unmapped variant by the payload of its answering GameAction) is the way to shrink this, not per-variant judgement.", - suggested_protocol_extension: "None needed upstream — the nineteen families already cover the great majority of the unmapped variants on payload shape alone. This is adapter work.", + reason: "Narrowed: this is no longer the wildcard arm's blanket answer. The wildcard now routes to interaction_prompt(), which serves any waiting state the engine projects as a finite ExactChoices list — the largest response class by far — so an unnamed WaitingFor is no longer unmapped by default. What remains here is the degenerate projection: no opportunity for this viewer, or an opportunity whose choice list is empty. Neither is a missing protocol shape; both mean there is nothing for this seat to answer, which is an engine or sequencing condition rather than a capability gap.", + suggested_protocol_extension: "None needed upstream. If this is observed while the seat genuinely owes a decision, it is an interaction-projection defect to fix, not a family to add.", + }, + UnsupportedCapability { + code: "local.interaction-simultaneous-decisions-unmapped", + area: "prompts", + reason: "The engine opens one interaction slot per semantic owner, and a single viewer can be the authorized submitter for more than one of them — a decision both seats owe at once, answered independently. The protocol carries one prompt per message and one answer per prompt, so there is no shape for 'here are two decisions, answer both'. Serving only the first would answer one seat and silently drop the other, which is why this fails closed instead. Note this is a projection-level count, not a mechanic: the same waiting state produces one opportunity in the ordinary case and lands here only when authority for several owners collapses onto one viewer.", + suggested_protocol_extension: "Either allow a batch of prompts to be outstanding for one recipient with independent prompt ids, or state that the server must serialize simultaneous decisions into successive prompts. The second needs no wire change and is likely the cheaper answer.", + }, + UnsupportedCapability { + code: "local.interaction-schema-response-unmapped", + area: "prompts", + reason: "The engine projects a decision either as a finite ExactChoices list or as a schema: a response spec plus candidates, covering sequences, grouped sequences, relations, mana groups, numbers, amount and damage assignments, deck partitions, text and shortcut replies. interaction_prompt() maps the finite class generically, because a materialized candidate list is exactly ChooseFromSelection's shape. The schema class has an unbounded response space that no single family expresses, so each spec needs its own family: Number -> ChooseNumber, Relations -> ChooseAttackers/ChooseBlockers, AssignDamage -> ChooseCombatDamageAssignment, Sequence -> Reorder or ChooseCards, DeckPartition -> ChooseCards. Those mappings are unwritten, so schema-valued decisions with no bespoke arm fail closed here rather than being flattened into a selection that would lose their bounds.", + suggested_protocol_extension: "None needed upstream for most specs — the families listed above already exist. GroupedSequence (per-group min/max) and ManaGroups are the two whose bounds no current family carries; resolve whether ChooseFromSelection should gain per-option group constraints before proposing new families.", }, UnsupportedCapability { code: "local.target-slot-missing", @@ -2308,7 +2343,7 @@ fn build_prompt_input( WaitingFor::CombatTaxPayment { .. } => { unsupported_prompt(waiting_for, "local.pay-combat-cost-unsupported") } - _ => unsupported_prompt(waiting_for, "local.prompt-unsupported"), + _ => interaction_prompt(prepared), } } @@ -2319,6 +2354,141 @@ fn unsupported_prompt(waiting_for: &WaitingFor, code: &'static str) -> Result }) } +/// Build a prompt from the engine's own interaction projection. +/// +/// The fallback for every waiting state with no bespoke arm above, and +/// deliberately generic. The engine classifies all of its waiting states into a +/// small set of response models, and for the finite ones it hands back concrete +/// labelled choices it has already validated. Hand-writing one mapping per +/// waiting state instead would re-derive bounds the engine has computed — game +/// logic duplicated inside a serialization boundary, and the exact drift the +/// interaction subsystem exists to prevent. +/// +/// Scope is `ExactChoices` only. A finite, pre-materialized candidate list is +/// precisely `ChooseFromSelection`'s shape, so the mapping is total and needs no +/// per-variant judgement. The schema-valued specs (sequences, numbers, amount +/// assignments, relations) carry an unbounded response space that no single +/// prompt family expresses; they still fail closed under a declared code. +fn interaction_prompt(prepared: &PreparedManabrewSnapshot) -> Result { + let waiting_for = &prepared.state.waiting_for; + // One opportunity per interaction slot this viewer may submit for, and a + // viewer can be the authorized submitter for more than one semantic owner — + // a simultaneous decision both seats owe. The wire carries a single prompt, + // so serving `.first()` would answer one seat and silently drop the other. + let [opportunity] = prepared.interaction.opportunities.as_slice() else { + return unsupported_prompt( + waiting_for, + if prepared.interaction.opportunities.is_empty() { + "local.prompt-unsupported" + } else { + "local.interaction-simultaneous-decisions-unmapped" + }, + ); + }; + let InteractionOpportunityResponse::ExactChoices { choices } = &opportunity.response else { + return unsupported_prompt(waiting_for, "local.interaction-schema-response-unmapped"); + }; + if choices.is_empty() { + return unsupported_prompt(waiting_for, "local.prompt-unsupported"); + } + Ok(PromptInput::ChooseFromSelection(ChooseFromSelectionInput { + presentation: presentation("Choose"), + options: choices + .iter() + .map(|choice| selection_option(choice_label(choice))) + .collect(), + // `ExactChoices` is a one-of list: the engine materialized each entry as + // a complete answer to the whole decision, so exactly one is chosen. + min_total: 1, + max_total: 1, + })) +} + +/// Answer a generically-projected prompt by handing the pick back to the engine. +/// +/// The index is positional into the same `ExactChoices` list `interaction_prompt` +/// rendered. That list is re-derived here rather than carried through +/// `PromptContext` because the projection is a pure function of state, and +/// staleness is already the prompt id's obligation — [`translate_response`] +/// rejects a mismatched id before reaching this point. +/// +/// The engine, not a local index→action table, turns the pick into a +/// `GameAction`. Response→action is game logic, and the engine's matcher is +/// exhaustive over its response models; a table built here would keep compiling +/// while silently going stale as models are added. +fn interaction_selection_action( + state: &GameState, + actor: PlayerId, + chosen_indices: &[usize], +) -> Result { + let illegal = |kind: &'static str| AdapterError::IllegalResponseForPrompt { + response_kind: kind, + }; + let [index] = chosen_indices else { + return Err(illegal( + "selectionDecision.chosenIndices expects exactly one pick", + )); + }; + let filtered = filter_state_for_viewer(state, actor); + let view = derive_viewer_interaction(state, &filtered, actor); + // Mirrors `interaction_prompt`'s guard: the prompt this answers was only + // ever built for a lone opportunity, so anything else means the projection + // moved and the echoed index no longer denotes what the client was shown. + let [opportunity] = view.opportunities.as_slice() else { + return Err(illegal( + "selectionDecision without exactly one open interaction", + )); + }; + let InteractionOpportunityResponse::ExactChoices { choices } = &opportunity.response else { + return Err(illegal( + "selectionDecision against a schema-valued interaction", + )); + }; + let choice = choices + .get(*index) + .ok_or_else(|| illegal("selectionDecision index outside the offered choices"))?; + resolve_interaction_response( + state, + actor, + &InteractionSubmission { + interaction_id: opportunity.interaction_id.clone(), + response: InteractionResponse::Choose { + choice_id: choice.id.clone(), + }, + }, + ) + .map_err(|_| illegal("selectionDecision the engine refused to materialize")) +} + +/// Label one projected choice, from the strings the engine already put on it. +/// +/// Every naming surface is joined rather than taking the first, because choices +/// in one list can share an object and differ only in a `Value` surface — the +/// priority projection offers auto-payment and manual-payment casts of the same +/// spell that way. Taking only the object name would render those two as the +/// same label, and the client picks by label even though it answers by index. +fn choice_label(choice: &InteractionChoice) -> String { + let parts = choice + .surfaces + .iter() + .filter_map(|surface| match surface { + InteractionPresentationSurface::Object { + name, reference, .. + } => Some(name.clone().unwrap_or_else(|| reference.clone())), + InteractionPresentationSurface::Value { value, .. } => Some(value.clone()), + InteractionPresentationSurface::Player { seat, .. } => Some(format!("Player {seat}")), + _ => None, + }) + .collect::>(); + if parts.is_empty() { + // No naming surface at all. The opaque id is a poor label but a correct + // one; it is never empty, so the option stays distinguishable. + choice.id.as_str().to_string() + } else { + parts.join(" — ") + } +} + impl PromptInput { /// The formal prompt/response contract: a response is valid only if its /// output family matches this prompt **and** every echoed action id was @@ -2582,9 +2752,18 @@ pub fn translate_response( } PromptOutput::ChooseFromSelection(ChooseFromSelectionOutput::SelectionDecision { chosen_indices, - }) => Ok(GameAction::SelectModes { - indices: chosen_indices, - }), + }) => match &state.waiting_for { + // The two bespoke producers of this family. Their answer is a list + // of mode indices — one response covering several picks — which is + // not the one-choice-per-answer shape the projection returns, so it + // cannot route through `ExactChoices`. + WaitingFor::ModeChoice { .. } | WaitingFor::AbilityModeChoice { .. } => { + Ok(GameAction::SelectModes { + indices: chosen_indices, + }) + } + _ => interaction_selection_action(state, context.deciding_player, &chosen_indices), + }, PromptOutput::ChooseColor(ChooseColorOutput::ColorDecision { chosen_colors }) => { translate_color_decision(&state.waiting_for, chosen_colors) } @@ -4058,10 +4237,20 @@ fn output_family_matches_waiting( WaitingFor::TargetSelection { .. } | WaitingFor::TriggerTargetSelection { .. } ), PromptOutput::ChooseNumber(_) => matches!(waiting_for, WaitingFor::ChooseXValue { .. }), - PromptOutput::ChooseFromSelection(_) => matches!( - waiting_for, - WaitingFor::ModeChoice { .. } | WaitingFor::AbilityModeChoice { .. } - ), + // The one family with no fixed `WaitingFor` list, because it is now + // reachable two ways: the two bespoke modal arms, and the generic + // projection path that serves any state the engine renders as a finite + // choice list. Enumerating the latter would reintroduce exactly the + // per-variant bookkeeping the projection removes, and would rot the + // moment the engine reclassifies a state. + // + // So ask the real question — would the prompt currently open be a + // `ChooseFromSelection`? — by consulting the builder itself. It cannot + // drift from the builder because it *is* the builder. Checking the + // projection alone would be wrong: `WaitingFor::Priority` also projects + // a finite list, and would then accept a `chooseFromSelection` answer to + // a `chooseAction` prompt. + PromptOutput::ChooseFromSelection(_) => open_prompt_is_generic_selection(state, viewer), PromptOutput::ChooseColor(_) => matches!(waiting_for, WaitingFor::ChooseManaColor { .. }), PromptOutput::ChooseCombatDamageAssignment(_) => { matches!(waiting_for, WaitingFor::AssignCombatDamage { .. }) @@ -4091,6 +4280,25 @@ fn output_family_matches_waiting( } /// The output's family tag, for diagnostics. +/// Would the prompt currently open for this viewer be a `ChooseFromSelection`? +/// +/// Rebuilds it rather than re-deriving the answer, so the gate and the prompt +/// can never disagree about which family is open. One extra prompt build per +/// answer is proportionate: this runs once per human decision. +/// +/// The lookup yields no card text on purpose. Family selection never depends on +/// it — the arms that need text belong to other families, and they fail with +/// `MissingCardText`, which is not `ChooseFromSelection` and so answers `false` +/// exactly as it should. +fn open_prompt_is_generic_selection(state: &GameState, viewer: PlayerId) -> bool { + prepare_snapshot(state, viewer, "").is_ok_and(|prepared| { + matches!( + build_prompt_input(&prepared, &(|_: &GameObject| -> Option { None })), + Ok(PromptInput::ChooseFromSelection(_)) + ) + }) +} + fn output_family(output: &PromptOutput) -> &'static str { match output { PromptOutput::Mulligan(_) => "mulligan", @@ -4591,6 +4799,7 @@ mod tests { use super::*; use std::collections::HashSet; + use engine::game::interaction::bind_interaction_authority; use engine::game::zones::create_object; use engine::types::ability::{Effect, ResolvedAbility, TargetFilter}; use engine::types::counter::CounterType; @@ -4599,6 +4808,7 @@ mod tests { TargetSelectionProgress, TargetSelectionSlot, }; use engine::types::identifiers::CardId; + use engine::types::interaction::InteractionSessionId; use pretty_assertions::assert_eq; fn lookup(_: &GameObject) -> Option { @@ -5493,6 +5703,96 @@ mod tests { )); } + /// The generic path: a waiting state with no bespoke arm is now prompted + /// from the engine's own projection instead of being refused. + /// + /// `TopOrBottomChoice` is chosen deliberately. It is one of the 85 variants + /// this adapter never names, and its projected choices differ only by a + /// `Value` surface — so this also pins that `choice_label` reads the + /// surfaces rather than falling back to the opaque choice id. + /// + /// Indices are compared by looking the label up rather than by assuming a + /// candidate order the engine never promised; the assertion that matters is + /// that the index the client echoes round-trips to the action that label + /// stands for. + #[test] + fn an_unmapped_waiting_state_prompts_from_the_interaction_projection() { + let mut state = GameState::new_two_player(7); + let object_id = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Scried Card".to_string(), + Zone::Library, + ); + state.waiting_for = WaitingFor::TopOrBottomChoice { + player: PlayerId(0), + object_id, + }; + bind_interaction_authority(&mut state, InteractionSessionId("generic-path".to_string())) + .expect("valid interaction authority binding"); + + let prepared = prepare_snapshot_with_prompt_id(&state, PlayerId(0), "game-a", 42).unwrap(); + let prompt = build_prompt_input(&prepared, &lookup) + .expect("an unmapped waiting state is served by the projection, not refused"); + let PromptInput::ChooseFromSelection(input) = prompt else { + panic!("a finite candidate list is ChooseFromSelection's shape, got {prompt:?}"); + }; + let labels = input + .options + .iter() + .map(|option| option.label.clone()) + .collect::>(); + assert!( + labels.iter().any(|label| label == "top") + && labels.iter().any(|label| label == "bottom"), + "the projection labels each choice from its Value surface, got {labels:?}" + ); + assert_eq!((input.min_total, input.max_total), (1, 1)); + + let top_index = labels.iter().position(|label| label == "top").unwrap(); + let action = translate_response( + 42, + PromptOutput::ChooseFromSelection(ChooseFromSelectionOutput::SelectionDecision { + chosen_indices: vec![top_index], + }), + &prepared.prompt_context(), + &state, + ) + .expect("the echoed index resolves back through the engine"); + assert_eq!(action, GameAction::ChooseTopOrBottom { top: true }); + } + + /// Without a bound interaction authority the projection is empty, so the + /// generic path cannot serve the prompt and the adapter must say so rather + /// than emit an option-less selection. This is also the non-vacuity guard + /// for the test above: it is the same waiting state, differing only in the + /// binding, so that test cannot be passing for an unrelated reason. + #[test] + fn an_unbound_interaction_authority_leaves_the_generic_path_unsupported() { + let mut state = GameState::new_two_player(7); + let object_id = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Scried Card".to_string(), + Zone::Library, + ); + state.waiting_for = WaitingFor::TopOrBottomChoice { + player: PlayerId(0), + object_id, + }; + + let prepared = prepare_snapshot_with_prompt_id(&state, PlayerId(0), "game-a", 42).unwrap(); + assert!(matches!( + build_prompt_input(&prepared, &lookup), + Err(AdapterError::UnsupportedPrompt { + code: "local.prompt-unsupported", + .. + }) + )); + } + // ------------------------------------------------------- wire shapes --- /// The core v2 change: `PromptOutput` is ADJACENTLY tagged, so the family's @@ -6672,11 +6972,18 @@ mod tests { object_id: ObjectId(4), }, ]; + let state = GameState::new_two_player(7); + let filtered = filter_state_for_viewer(&state, PlayerId(0)); let prepared = PreparedManabrewSnapshot { game_id: "game-a".to_string(), viewer: PlayerId(0), prompt_id: 7, - state: GameState::new_two_player(7), + // A real projection rather than a stand-in. This state has no bound + // interaction authority, so it comes back empty — which is correct + // and irrelevant here: the assertions below concern the payment + // action id space, which `pay_mana_cost_input` reads from `actions`. + interaction: derive_viewer_interaction(&state, &filtered, PlayerId(0)), + state, derived: DerivedViews::default(), actions: actions.clone(), spell_costs: HashMap::new(), @@ -7132,13 +7439,13 @@ mod tests { #[test] fn unsupported_capability_registry_is_well_formed() { let capabilities = unsupported_protocol_capabilities(); - assert_eq!(capabilities.len(), 79); + assert_eq!(capabilities.len(), 81); let codes: HashSet<_> = capabilities .iter() .map(|capability| capability.code) .collect(); - assert_eq!(codes.len(), 79, "capability codes must be unique"); + assert_eq!(codes.len(), 81, "capability codes must be unique"); for capability in capabilities { assert!( From be257da2cc8faed5e83d45ca515821b9f7341183 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Mon, 27 Jul 2026 11:40:57 -0700 Subject: [PATCH 2/4] feat(manabrew-compat): serve Select schemas through the generic prompt path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The projection path handled one-of lists (`ExactChoices`). This adds the other half of the same idea: `Select`, a subset choice over the same kind of candidate list, differing only in how many entries may be taken. That difference is exactly what `ChooseFromSelection` already carries. `SelectionConstraint::Count` and `EngineValidatedCount` both bound a count, so they become `min_total`/`max_total` verbatim instead of the one-of path's hardcoded 1/1. `EngineValidatedCount` is treated identically on purpose: the extra legality the engine reserves to itself is rechecked on submit and is not expressible to a client either way, so advertising the count is the whole of what this family can honestly say. `SelectionConstraint::Aggregate` is not folded in. Its bound is a sum over a chosen attribute of the selected objects — "keep permanents with total power 4 or less" — not a number of objects, so no count is equivalent to it. Rendering it as an unbounded count would advertise illegal selections as legal, which is worse than refusing, so it fails closed under its own code. The suggested extension is cheap and worth recording: `SelectionOption` already carries `weight`, so the wire is one comparator and one amount away from expressing this without a new family. The response variant is not interchangeable with the spec — the engine rejects a `Choose` submitted against a `Select` schema as malformed, and vice versa — so the answer path now mirrors whichever shape the prompt rendered. Count bounds are deliberately not rechecked adapter-side: the engine owns them and rejects a violating submission, and a second check here would be a drifting authority on the same constraint. Measured over the production half of the file, prompt coverage goes from 94/127 `WaitingFor` variants to **113/127**. The 14 that remain are `TargetSequence` (6), `AmountAssignments` (3), and five singletons; each needs a family whose payload is not a count over a candidate list at all — an ordering, an amount distribution, a partition — so none of them belongs in this family. Note the census counts by response *model*: a `Select` whose runtime constraint is `Aggregate` still fails closed, so 113 is an upper bound. The new test uses `DiscardToHandSize` (CR 514.1) because it is the clearest subset choice — discard exactly `count` of the cards in hand — so it pins both things that distinguish this path from the one-of path: that the engine's bounds survive into the prompt, and that the answer goes back as a subset. 77 -> 78 tests, all passing, with `cargo clippy -p manabrew-compat` clean — run in an isolated `CARGO_TARGET_DIR`, since no Tilt resource executes this crate's tests. --- crates/manabrew-compat/src/lib.rs | 171 +++++++++++++++++++++++++----- 1 file changed, 144 insertions(+), 27 deletions(-) diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index aff2aee3f0..09e4492e80 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -28,7 +28,8 @@ use engine::types::game_state::{ }; use engine::types::interaction::{ InteractionChoice, InteractionOpportunityResponse, InteractionPresentationSurface, - InteractionResponse, InteractionSubmission, ViewerInteraction, + InteractionResponse, InteractionResponseSpec, InteractionSubmission, SelectionConstraint, + ViewerInteraction, }; use engine::types::mana::{ManaColor as EngineManaColor, ManaCost, ManaCostShard, ManaType}; use engine::types::phase::Phase; @@ -1358,7 +1359,7 @@ pub fn unsupported_protocol_capabilities() -> &'static [UnsupportedCapability] { /// /// `upstream.` = the protocol has no primitive for something the engine can do. /// `local.` = the protocol has the primitive but this engine cannot source it. -static UNSUPPORTED_PROTOCOL_CAPABILITIES: [UnsupportedCapability; 81] = [ +static UNSUPPORTED_PROTOCOL_CAPABILITIES: [UnsupportedCapability; 82] = [ UnsupportedCapability { code: "upstream.object-selection-missing", area: "prompts", @@ -1561,8 +1562,14 @@ static UNSUPPORTED_PROTOCOL_CAPABILITIES: [UnsupportedCapability; 81] = [ UnsupportedCapability { code: "local.interaction-schema-response-unmapped", area: "prompts", - reason: "The engine projects a decision either as a finite ExactChoices list or as a schema: a response spec plus candidates, covering sequences, grouped sequences, relations, mana groups, numbers, amount and damage assignments, deck partitions, text and shortcut replies. interaction_prompt() maps the finite class generically, because a materialized candidate list is exactly ChooseFromSelection's shape. The schema class has an unbounded response space that no single family expresses, so each spec needs its own family: Number -> ChooseNumber, Relations -> ChooseAttackers/ChooseBlockers, AssignDamage -> ChooseCombatDamageAssignment, Sequence -> Reorder or ChooseCards, DeckPartition -> ChooseCards. Those mappings are unwritten, so schema-valued decisions with no bespoke arm fail closed here rather than being flattened into a selection that would lose their bounds.", - suggested_protocol_extension: "None needed upstream for most specs — the families listed above already exist. GroupedSequence (per-group min/max) and ManaGroups are the two whose bounds no current family carries; resolve whether ChooseFromSelection should gain per-option group constraints before proposing new families.", + reason: "The engine projects a decision either as a finite ExactChoices list or as a schema: a response spec plus candidates. interaction_prompt() maps two of those generically — ExactChoices (a one-of list) and Select (a subset choice, whose count bounds ChooseFromSelection's min/max totals express exactly). The rest still fail closed here, because their response space is not a count over a candidate list and flattening it into one would advertise illegal answers as legal. Each needs its own family and none of those mappings are written yet: Number -> ChooseNumber, Relations -> ChooseAttackers/ChooseBlockers, AssignDamage -> ChooseCombatDamageAssignment, AssignAmounts -> ChooseCombatDamageAssignment's shape but for counters, Sequence -> Reorder (ordering is the payload, not membership), DeckPartition -> ChooseCards, Text/Shortcut/ShortcutReply -> no current family at all.", + suggested_protocol_extension: "None needed upstream for Number, Relations, AssignDamage, Sequence or DeckPartition — those families already exist and this is adapter work. GroupedSequence (per-group min/max), ManaGroups, and the Text/Shortcut pair are the ones whose payload no current family carries; resolve whether ChooseFromSelection should gain per-option group constraints before proposing new families.", + }, + UnsupportedCapability { + code: "local.interaction-aggregate-bound-unmapped", + area: "prompts", + reason: "A Select schema whose SelectionConstraint is Aggregate rather than Count. The bound is a sum over a chosen attribute of the selected objects — 'keep permanents with total power 4 or less' — not a number of objects, so no min/max count is equivalent to it. ChooseFromSelection carries min_total/max_total as counts only, and rendering an aggregate bound as an unbounded count would advertise illegal selections as legal, which is worse than refusing. Distinct from local.interaction-schema-response-unmapped because the spec IS mapped: only this one constraint variant within it is not.", + suggested_protocol_extension: "Give ChooseFromSelection an optional aggregate bound over a named option weight — SelectionOption already carries `weight`, so the wire is one comparator and one amount away from expressing this without a new family.", }, UnsupportedCapability { code: "local.target-slot-missing", @@ -2385,8 +2392,39 @@ fn interaction_prompt(prepared: &PreparedManabrewSnapshot) -> Result (choices, 1, 1), + // A subset choice over the same kind of candidate list, differing only + // in how many may be taken — which the constraint carries, so + // `ChooseFromSelection`'s min/max totals express it exactly. + InteractionOpportunityResponse::Schema { + spec: InteractionResponseSpec::Select { constraint, .. }, + candidates, + } => match constraint { + // `EngineValidatedCount` bounds the count identically. The extra + // legality the engine reserves to itself is rechecked on submit and + // is not expressible to a client either way, so advertising the + // count is the whole of what this family can honestly say. + SelectionConstraint::Count { min, max } + | SelectionConstraint::EngineValidatedCount { min, max } => { + (candidates, *min as usize, *max as usize) + } + // An aggregate bound — "keep permanents with total power 4 or less" + // — constrains a sum over a chosen attribute, not a count. No family + // carries it, and flattening it to an unbounded count would + // advertise illegal answers as legal. + SelectionConstraint::Aggregate { .. } => { + return unsupported_prompt( + waiting_for, + "local.interaction-aggregate-bound-unmapped", + ) + } + }, + InteractionOpportunityResponse::Schema { .. } => { + return unsupported_prompt(waiting_for, "local.interaction-schema-response-unmapped") + } }; if choices.is_empty() { return unsupported_prompt(waiting_for, "local.prompt-unsupported"); @@ -2397,10 +2435,8 @@ fn interaction_prompt(prepared: &PreparedManabrewSnapshot) -> Result { + let [index] = chosen_indices else { + return Err(illegal( + "selectionDecision over a one-of list expects exactly one pick", + )); + }; + InteractionResponse::Choose { + choice_id: id_at(index, choices)?, + } + } + InteractionOpportunityResponse::Schema { + spec: InteractionResponseSpec::Select { .. }, + candidates, + } => InteractionResponse::Select { + // Count bounds are not rechecked here. The engine owns them and + // rejects a violating submission; duplicating the check would put a + // second, drifting authority on the same constraint. + choice_ids: chosen_indices + .iter() + .map(|index| id_at(index, candidates)) + .collect::>>()?, + }, + InteractionOpportunityResponse::Schema { .. } => { + return Err(illegal( + "selectionDecision against a schema this family cannot express", + )) + } }; - let choice = choices - .get(*index) - .ok_or_else(|| illegal("selectionDecision index outside the offered choices"))?; resolve_interaction_response( state, actor, &InteractionSubmission { interaction_id: opportunity.interaction_id.clone(), - response: InteractionResponse::Choose { - choice_id: choice.id.clone(), - }, + response, }, ) .map_err(|_| illegal("selectionDecision the engine refused to materialize")) @@ -5763,6 +5821,65 @@ mod tests { assert_eq!(action, GameAction::ChooseTopOrBottom { top: true }); } + /// The `Select` half of the generic path: a subset choice, not a one-of. + /// + /// `DiscardToHandSize` (CR 514.1) is the clearest case — discard exactly + /// `count` of the cards in hand — so it pins the two things that distinguish + /// this from the `ExactChoices` path: the count bounds reach the prompt as + /// `min_total`/`max_total` instead of the hardcoded 1/1, and the answer must + /// go back as `InteractionResponse::Select`, since the engine rejects a + /// `Choose` against a `Select` schema as malformed. + #[test] + fn a_select_schema_carries_its_count_bounds_and_answers_as_a_subset() { + let mut state = GameState::new_two_player(7); + let cards = ["Discard A", "Discard B", "Discard C"] + .into_iter() + .map(|name| { + create_object( + &mut state, + CardId(1), + PlayerId(0), + name.to_string(), + Zone::Hand, + ) + }) + .collect::>(); + state.waiting_for = WaitingFor::DiscardToHandSize { + player: PlayerId(0), + count: 2, + cards: cards.clone(), + }; + bind_interaction_authority(&mut state, InteractionSessionId("select-path".to_string())) + .expect("valid interaction authority binding"); + + let prepared = prepare_snapshot_with_prompt_id(&state, PlayerId(0), "game-a", 42).unwrap(); + let prompt = build_prompt_input(&prepared, &lookup) + .expect("a Select schema is served by the projection"); + let PromptInput::ChooseFromSelection(input) = prompt else { + panic!("a subset choice over candidates is ChooseFromSelection, got {prompt:?}"); + }; + assert_eq!( + (input.min_total, input.max_total), + (2, 2), + "the engine's count bounds must survive, not the one-of path's 1/1" + ); + assert_eq!(input.options.len(), 3, "every hand card is a candidate"); + + let action = translate_response( + 42, + PromptOutput::ChooseFromSelection(ChooseFromSelectionOutput::SelectionDecision { + chosen_indices: vec![0, 1], + }), + &prepared.prompt_context(), + &state, + ) + .expect("a two-card subset resolves back through the engine"); + assert!( + matches!(action, GameAction::SelectCards { .. }), + "a discard subset answers with SelectCards, got {action:?}" + ); + } + /// Without a bound interaction authority the projection is empty, so the /// generic path cannot serve the prompt and the adapter must say so rather /// than emit an option-less selection. This is also the non-vacuity guard @@ -7439,13 +7556,13 @@ mod tests { #[test] fn unsupported_capability_registry_is_well_formed() { let capabilities = unsupported_protocol_capabilities(); - assert_eq!(capabilities.len(), 81); + assert_eq!(capabilities.len(), 82); let codes: HashSet<_> = capabilities .iter() .map(|capability| capability.code) .collect(); - assert_eq!(codes.len(), 81, "capability codes must be unique"); + assert_eq!(codes.len(), 82, "capability codes must be unique"); for capability in capabilities { assert!( From 5393ec2ef0326060e7ff37d02f6e301718c169fd Mon Sep 17 00:00:00 2001 From: matthewevans Date: Mon, 27 Jul 2026 11:53:02 -0700 Subject: [PATCH 3/4] feat(manabrew-compat): serve Number schemas as ChooseNumber MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generic path so far emitted one family, because both shapes it handled were a choice over candidates. `Number` is the first that is not: a range with no candidate list at all, which `ChooseNumber` carries verbatim. Handled before the candidate branches precisely because its candidate list is empty by construction — the emptiness guard that protects the selection path would otherwise reject it. The answering action is the engine's to name, and this is where the bespoke arm would have been wrong. `GameAction::ChooseX` is specific to X (CR 107.3); the one unmapped numeric pause is `PayAmountChoice` (CR 107.14, pay any amount of `{E}`), which the engine answers with `SubmitPayAmount`. So the arm now dispatches `ChooseXValue` to `ChooseX` as before and routes everything else through the projection, which resolves it to whatever action the engine actually names. The test asserts `SubmitPayAmount { amount: 2 }` rather than merely "not ChooseX", so a regression to the wrong action fails loudly. Value bounds are deliberately not rechecked adapter-side, for the same reason the selection path does not recheck counts: the engine owns the range and rejects a violating submission, and a second check here would be a drifting authority on the same constraint. Two pieces of shared structure fall out and are factored rather than copied: * `sole_open_opportunity` — every generic response path needs the lone open interaction, and the "exactly one" rule is the same guard for all of them. * `open_prompt` — the gate for every family the generic path can emit. Those families have no fixed `WaitingFor` list, so the gate rebuilds the prompt and reads its family; it cannot drift from the builder because it *is* the builder. `ChooseNumber` now joins `ChooseFromSelection` in using it, with the bespoke `ChooseXValue` check kept as the primary so shipped behaviour for X is untouched. Prompt coverage: 113/127 -> **114/127**. The 13 that remain are `TargetSequence` (6), `AmountAssignments` (3), and singletons for sideboarding, category choice, and the two shortcut families. None is a count over a list or a scalar; each needs a family whose payload is an ordering, a distribution, or a partition, so none belongs in the two shapes already served. Also corrects an orphaned doc comment: an earlier edit left `/// The output's family tag, for diagnostics.` stranded above the wrong item. 79 tests passing, `cargo clippy -p manabrew-compat` clean, both in an isolated `CARGO_TARGET_DIR` since no Tilt resource runs this crate's tests. --- crates/manabrew-compat/src/lib.rs | 209 +++++++++++++++++++++++++----- 1 file changed, 174 insertions(+), 35 deletions(-) diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index 09e4492e80..1e470b28c8 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -27,9 +27,9 @@ use engine::types::game_state::{ ShardChoice, StackEntryKind, WaitingFor, }; use engine::types::interaction::{ - InteractionChoice, InteractionOpportunityResponse, InteractionPresentationSurface, - InteractionResponse, InteractionResponseSpec, InteractionSubmission, SelectionConstraint, - ViewerInteraction, + InteractionChoice, InteractionOpportunity, InteractionOpportunityResponse, + InteractionPresentationSurface, InteractionResponse, InteractionResponseSpec, + InteractionSubmission, SelectionConstraint, ViewerInteraction, }; use engine::types::mana::{ManaColor as EngineManaColor, ManaCost, ManaCostShard, ManaType}; use engine::types::phase::Phase; @@ -1562,8 +1562,8 @@ static UNSUPPORTED_PROTOCOL_CAPABILITIES: [UnsupportedCapability; 82] = [ UnsupportedCapability { code: "local.interaction-schema-response-unmapped", area: "prompts", - reason: "The engine projects a decision either as a finite ExactChoices list or as a schema: a response spec plus candidates. interaction_prompt() maps two of those generically — ExactChoices (a one-of list) and Select (a subset choice, whose count bounds ChooseFromSelection's min/max totals express exactly). The rest still fail closed here, because their response space is not a count over a candidate list and flattening it into one would advertise illegal answers as legal. Each needs its own family and none of those mappings are written yet: Number -> ChooseNumber, Relations -> ChooseAttackers/ChooseBlockers, AssignDamage -> ChooseCombatDamageAssignment, AssignAmounts -> ChooseCombatDamageAssignment's shape but for counters, Sequence -> Reorder (ordering is the payload, not membership), DeckPartition -> ChooseCards, Text/Shortcut/ShortcutReply -> no current family at all.", - suggested_protocol_extension: "None needed upstream for Number, Relations, AssignDamage, Sequence or DeckPartition — those families already exist and this is adapter work. GroupedSequence (per-group min/max), ManaGroups, and the Text/Shortcut pair are the ones whose payload no current family carries; resolve whether ChooseFromSelection should gain per-option group constraints before proposing new families.", + reason: "The engine projects a decision either as a finite ExactChoices list or as a schema: a response spec plus candidates. interaction_prompt() maps three of those generically — ExactChoices (a one-of list), Select (a subset choice, whose count bounds ChooseFromSelection's min/max totals express exactly), and Number (a range, which is ChooseNumber verbatim). The rest still fail closed here, because their payload is not a count over a candidate list nor a scalar, and flattening one into either would advertise illegal answers as legal. Each needs its own family and none of those mappings are written yet: Sequence -> Reorder (ordering is the payload, not membership), Relations -> ChooseAttackers/ChooseBlockers, AssignDamage -> ChooseCombatDamageAssignment, DeckPartition -> ChooseCards. AssignAmounts is a distribution of a total across candidates, which resembles ChooseCombatDamageAssignment but is not damage, so reusing that family would misname it. GroupedSequence, ManaGroups, Text, Shortcut and ShortcutReply have no current family at all.", + suggested_protocol_extension: "None needed upstream for Sequence, Relations, AssignDamage or DeckPartition — those families already exist and this is adapter work. GroupedSequence (per-group min/max), ManaGroups, AssignAmounts (a non-damage distribution), and the Text/Shortcut pair are the ones whose payload no current family carries; resolve whether ChooseFromSelection should gain per-option group constraints and a generic distribution shape before proposing new families.", }, UnsupportedCapability { code: "local.interaction-aggregate-bound-unmapped", @@ -2392,6 +2392,24 @@ fn interaction_prompt(prepared: &PreparedManabrewSnapshot) -> Result Result { + let filtered = filter_state_for_viewer(state, actor); + let mut view = derive_viewer_interaction(state, &filtered, actor); + if view.opportunities.len() != 1 { + return Err(AdapterError::IllegalResponseForPrompt { + response_kind: "a response without exactly one open interaction", + }); + } + Ok(view.opportunities.remove(0)) +} + +/// Answer a generically-projected numeric prompt. +/// +/// Split from the selection path because the two share no payload: this one +/// carries a value, not indices into a candidate list. What they do share — +/// finding the lone open opportunity, and letting the engine name the answering +/// action — lives in [`sole_open_opportunity`] and +/// [`resolve_interaction_response`]. +fn interaction_number_action(state: &GameState, actor: PlayerId, value: u32) -> Result { + let illegal = |kind: &'static str| AdapterError::IllegalResponseForPrompt { + response_kind: kind, + }; + let opportunity = sole_open_opportunity(state, actor)?; + if !matches!( + opportunity.response, + InteractionOpportunityResponse::Schema { + spec: InteractionResponseSpec::Number { .. }, + .. + } + ) { + return Err(illegal( + "numberDecision against an interaction that is not a numeric range", + )); + } + // The engine range-checks the value; re-checking it here would be a second + // authority on the same bound, free to drift from the one that decides. + resolve_interaction_response( + state, + actor, + &InteractionSubmission { + interaction_id: opportunity.interaction_id, + response: InteractionResponse::Number { value }, + }, + ) + .map_err(|_| illegal("numberDecision the engine refused to materialize")) +} + /// Label one projected choice, from the strings the engine already put on it. /// /// Every naming surface is joined rather than taking the first, because choices @@ -2800,9 +2862,15 @@ pub fn translate_response( // CR 107.3 + CR 107.1b: X is a value its controller chooses, and // a negative number can never be chosen — so a declined or // negative answer is not a legal X. - Some(value) if value >= 0 => Ok(GameAction::ChooseX { - value: value as u32, - }), + Some(value) if value >= 0 => match &state.waiting_for { + WaitingFor::ChooseXValue { .. } => Ok(GameAction::ChooseX { + value: value as u32, + }), + // Every other numeric pause reaches the client through the + // projection, and its answering action is the engine's to + // name — `ChooseX` is specific to X, not to numbers. + _ => interaction_number_action(state, context.deciding_player, value as u32), + }, _ => Err(AdapterError::IllegalResponseForPrompt { response_kind: "numberDecision", }), @@ -4294,7 +4362,12 @@ fn output_family_matches_waiting( waiting_for, WaitingFor::TargetSelection { .. } | WaitingFor::TriggerTargetSelection { .. } ), - PromptOutput::ChooseNumber(_) => matches!(waiting_for, WaitingFor::ChooseXValue { .. }), + // Like `ChooseFromSelection`, reachable both bespoke (X, CR 107.3) and + // generically, so the open-prompt check carries it rather than a list. + PromptOutput::ChooseNumber(_) => { + matches!(waiting_for, WaitingFor::ChooseXValue { .. }) + || open_prompt_is_generic_number(state, viewer) + } // The one family with no fixed `WaitingFor` list, because it is now // reachable two ways: the two bespoke modal arms, and the generic // projection path that serves any state the engine renders as a finite @@ -4337,26 +4410,38 @@ fn output_family_matches_waiting( } } -/// The output's family tag, for diagnostics. -/// Would the prompt currently open for this viewer be a `ChooseFromSelection`? +/// Rebuild the prompt currently open for this viewer, to ask which family it is. /// -/// Rebuilds it rather than re-deriving the answer, so the gate and the prompt -/// can never disagree about which family is open. One extra prompt build per -/// answer is proportionate: this runs once per human decision. +/// The gate for every family the generic path can emit. Those families have no +/// fixed `WaitingFor` list — the projection decides — and enumerating one would +/// reintroduce exactly the per-variant bookkeeping the projection removes. +/// Rebuilding cannot drift from the builder because it *is* the builder. One +/// extra prompt build per answer is proportionate: this runs once per decision. /// /// The lookup yields no card text on purpose. Family selection never depends on -/// it — the arms that need text belong to other families, and they fail with -/// `MissingCardText`, which is not `ChooseFromSelection` and so answers `false` -/// exactly as it should. +/// it — the arms that need text belong to other families and fail with +/// `MissingCardText`, which is no family at all and so gates `false`, exactly as +/// it should. +fn open_prompt(state: &GameState, viewer: PlayerId) -> Option { + let prepared = prepare_snapshot(state, viewer, "").ok()?; + build_prompt_input(&prepared, &(|_: &GameObject| -> Option { None })).ok() +} + fn open_prompt_is_generic_selection(state: &GameState, viewer: PlayerId) -> bool { - prepare_snapshot(state, viewer, "").is_ok_and(|prepared| { - matches!( - build_prompt_input(&prepared, &(|_: &GameObject| -> Option { None })), - Ok(PromptInput::ChooseFromSelection(_)) - ) - }) + matches!( + open_prompt(state, viewer), + Some(PromptInput::ChooseFromSelection(_)) + ) } +fn open_prompt_is_generic_number(state: &GameState, viewer: PlayerId) -> bool { + matches!( + open_prompt(state, viewer), + Some(PromptInput::ChooseNumber(_)) + ) +} + +/// The output's family tag, for diagnostics. fn output_family(output: &PromptOutput) -> &'static str { match output { PromptOutput::Mulligan(_) => "mulligan", @@ -4862,8 +4947,8 @@ mod tests { use engine::types::ability::{Effect, ResolvedAbility, TargetFilter}; use engine::types::counter::CounterType; use engine::types::game_state::{ - MulliganDecisionEntry, MulliganDecisionPhase, PendingCast, PendingMulliganAction, - TargetSelectionProgress, TargetSelectionSlot, + MulliganDecisionEntry, MulliganDecisionPhase, PayableResource, PendingCast, + PendingMulliganAction, TargetSelectionProgress, TargetSelectionSlot, }; use engine::types::identifiers::CardId; use engine::types::interaction::InteractionSessionId; @@ -5880,6 +5965,60 @@ mod tests { ); } + /// A `Number` schema leaves the selection family entirely. + /// + /// `PayAmountChoice` (CR 107.14 — pay any amount of `{E}`) is the only + /// unmapped numeric pause. It pins two things: the engine's range reaches + /// the client as `ChooseNumber`'s bounds, and the answer resolves to the + /// action the *engine* names. That second half is the point — + /// `GameAction::ChooseX` is specific to X (CR 107.3), so the bespoke arm + /// would have answered this pause with the wrong action entirely. + #[test] + fn a_number_schema_becomes_choose_number_and_resolves_to_the_engines_action() { + let mut state = GameState::new_two_player(7); + let source_id = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Energy Sink".to_string(), + Zone::Battlefield, + ); + state.waiting_for = WaitingFor::PayAmountChoice { + player: PlayerId(0), + resource: PayableResource::Energy, + min: 0, + max: 3, + accumulated: 0, + source_id, + pending_mana_ability: None, + }; + bind_interaction_authority(&mut state, InteractionSessionId("number-path".to_string())) + .expect("valid interaction authority binding"); + + let prepared = prepare_snapshot_with_prompt_id(&state, PlayerId(0), "game-a", 42).unwrap(); + let prompt = build_prompt_input(&prepared, &lookup) + .expect("a Number schema is served by the projection"); + let PromptInput::ChooseNumber(input) = prompt else { + panic!("a numeric range is ChooseNumber, not a selection, got {prompt:?}"); + }; + assert_eq!( + (input.min, input.max), + (0, 3), + "the engine's range must survive into the prompt" + ); + + let action = translate_response( + 42, + PromptOutput::ChooseNumber(ChooseNumberOutput::NumberDecision { + chosen_number: Some(2), + }), + &prepared.prompt_context(), + &state, + ) + .expect("the chosen number resolves back through the engine"); + assert_eq!(action, GameAction::SubmitPayAmount { amount: 2 }); + } + /// Without a bound interaction authority the projection is empty, so the /// generic path cannot serve the prompt and the adapter must say so rather /// than emit an option-less selection. This is also the non-vacuity guard From 05efb8b6e0f6c83c3bc4466ca38adcce1c7bc776 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Mon, 27 Jul 2026 12:06:43 -0700 Subject: [PATCH 4/4] feat(manabrew-compat): serve Sequence schemas, preserving client order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Sequence` is an ordered subset of the same candidate list `Select` draws from, and `ChooseFromSelection` already carries order: `chosen_indices` is a `Vec`, not a set, so the sequence the client sends survives to the engine, which fills its target slots in exactly that order. It answers with `InteractionResponse::Sequence`, not `Select` — the engine rejects the wrong variant as malformed — so the response path mirrors whichever shape the prompt rendered, as it already does for the one-of case. Fidelity gap recorded rather than hidden: this family cannot *tell* the client that order is significant; it renders as a plain selection. `Reorder` is not a substitute, because it orders the whole list while a target sequence is usually a proper subset. The test reverses the offered order on purpose. That is the entire assertion — an implementation that collected indices into a set, or sorted them, would hand the engine the targets the other way round and fail. It uses `ProliferateChoice` (CR 701.27), which also pins that a zero minimum reaches the prompt intact instead of being coerced to the one-of path's 1. Prompt coverage: 114/127 -> **120/127**. The 7 that remain are `AmountAssignments` (3), and singletons for sideboarding, category choice, and the two shortcut families. Each needs a payload that is not a count over a list, an order over a list, or a scalar — a per-candidate distribution, a grouped partition, or a shortcut reply — so none belongs in the shapes served here. The registry entry now names the two protocol shapes that would close most of them. 80 tests passing, `cargo clippy -p manabrew-compat` clean with zero warnings, both in an isolated `CARGO_TARGET_DIR` since no Tilt resource runs this crate's tests. --- crates/manabrew-compat/src/lib.rs | 95 ++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/crates/manabrew-compat/src/lib.rs b/crates/manabrew-compat/src/lib.rs index 1e470b28c8..6d2ab1279d 100644 --- a/crates/manabrew-compat/src/lib.rs +++ b/crates/manabrew-compat/src/lib.rs @@ -1562,8 +1562,8 @@ static UNSUPPORTED_PROTOCOL_CAPABILITIES: [UnsupportedCapability; 82] = [ UnsupportedCapability { code: "local.interaction-schema-response-unmapped", area: "prompts", - reason: "The engine projects a decision either as a finite ExactChoices list or as a schema: a response spec plus candidates. interaction_prompt() maps three of those generically — ExactChoices (a one-of list), Select (a subset choice, whose count bounds ChooseFromSelection's min/max totals express exactly), and Number (a range, which is ChooseNumber verbatim). The rest still fail closed here, because their payload is not a count over a candidate list nor a scalar, and flattening one into either would advertise illegal answers as legal. Each needs its own family and none of those mappings are written yet: Sequence -> Reorder (ordering is the payload, not membership), Relations -> ChooseAttackers/ChooseBlockers, AssignDamage -> ChooseCombatDamageAssignment, DeckPartition -> ChooseCards. AssignAmounts is a distribution of a total across candidates, which resembles ChooseCombatDamageAssignment but is not damage, so reusing that family would misname it. GroupedSequence, ManaGroups, Text, Shortcut and ShortcutReply have no current family at all.", - suggested_protocol_extension: "None needed upstream for Sequence, Relations, AssignDamage or DeckPartition — those families already exist and this is adapter work. GroupedSequence (per-group min/max), ManaGroups, AssignAmounts (a non-damage distribution), and the Text/Shortcut pair are the ones whose payload no current family carries; resolve whether ChooseFromSelection should gain per-option group constraints and a generic distribution shape before proposing new families.", + reason: "The engine projects a decision either as a finite ExactChoices list or as a schema: a response spec plus candidates. interaction_prompt() now maps four generically — ExactChoices (a one-of list), Select (an unordered subset, whose count bounds ChooseFromSelection's min/max totals express exactly), Sequence (an ordered subset; chosen_indices is itself ordered, so the order survives), and Number (a range, which is ChooseNumber verbatim). What remains fails closed because its payload is none of those things: not a count over a list, not an order over a list, not a scalar. AssignAmounts distributes a total across candidates — a per-candidate amount, which ChooseCombatDamageAssignment shapes but names as damage, so reusing it would misdescribe counter distribution. GroupedSequence carries per-group min/max, DeckPartition splits a pool in two, and ManaGroups, Text, Shortcut and ShortcutReply have no current family at all. Flattening any of them into a selection would drop the very constraint that makes the answer legal.", + suggested_protocol_extension: "Two shapes would close most of it: a per-candidate amount distribution with a required total (covers AssignAmounts, and generalizes ChooseCombatDamageAssignment rather than competing with it), and per-option group constraints on ChooseFromSelection (covers GroupedSequence, and DeckPartition as the two-group case). The Text and Shortcut families are genuinely absent and need their own design conversation.", }, UnsupportedCapability { code: "local.interaction-aggregate-bound-unmapped", @@ -2440,6 +2440,18 @@ fn interaction_prompt(prepared: &PreparedManabrewSnapshot) -> Result (candidates, *min as usize, *max as usize), InteractionOpportunityResponse::Schema { .. } => { return unsupported_prompt(waiting_for, "local.interaction-schema-response-unmapped") } @@ -2510,6 +2522,18 @@ fn interaction_selection_action( .map(|index| id_at(index, candidates)) .collect::>>()?, }, + // Distinct from `Select` on the wire even though the prompt looks the + // same: the engine fills its slots in the order given, so the indices + // must stay in the order the client sent them. + InteractionOpportunityResponse::Schema { + spec: InteractionResponseSpec::Sequence { .. }, + candidates, + } => InteractionResponse::Sequence { + choice_ids: chosen_indices + .iter() + .map(|index| id_at(index, candidates)) + .collect::>>()?, + }, InteractionOpportunityResponse::Schema { .. } => { return Err(illegal( "selectionDecision against a schema this family cannot express", @@ -6019,6 +6043,73 @@ mod tests { assert_eq!(action, GameAction::SubmitPayAmount { amount: 2 }); } + /// A `Sequence` schema is an *ordered* subset, and the order must survive. + /// + /// `ProliferateChoice` (CR 701.27) projects min 0 / max = eligible count, so + /// it also pins that a zero minimum reaches the prompt intact rather than + /// being coerced to the one-of path's 1. + /// + /// The answer deliberately reverses the offered order. That is the whole + /// assertion: the engine fills its slots in the order the client sent, so a + /// path that collected indices into a set — or sorted them — would return + /// the targets the other way round and fail here. + #[test] + fn a_sequence_schema_preserves_the_order_the_client_sent() { + let mut state = GameState::new_two_player(7); + let first = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Counter Holder A".to_string(), + Zone::Battlefield, + ); + let second = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Counter Holder B".to_string(), + Zone::Battlefield, + ); + state.waiting_for = WaitingFor::ProliferateChoice { + player: PlayerId(0), + eligible: vec![TargetRef::Object(first), TargetRef::Object(second)], + }; + bind_interaction_authority( + &mut state, + InteractionSessionId("sequence-path".to_string()), + ) + .expect("valid interaction authority binding"); + + let prepared = prepare_snapshot_with_prompt_id(&state, PlayerId(0), "game-a", 42).unwrap(); + let prompt = build_prompt_input(&prepared, &lookup) + .expect("a Sequence schema is served by the projection"); + let PromptInput::ChooseFromSelection(input) = prompt else { + panic!("an ordered subset still renders as ChooseFromSelection, got {prompt:?}"); + }; + assert_eq!( + (input.min_total, input.max_total), + (0, 2), + "proliferate is optional, so the zero minimum must survive" + ); + + let action = translate_response( + 42, + PromptOutput::ChooseFromSelection(ChooseFromSelectionOutput::SelectionDecision { + chosen_indices: vec![1, 0], + }), + &prepared.prompt_context(), + &state, + ) + .expect("an ordered subset resolves back through the engine"); + assert_eq!( + action, + GameAction::SelectTargets { + targets: vec![TargetRef::Object(second), TargetRef::Object(first)], + }, + "the engine must receive the targets in the order the client chose" + ); + } + /// Without a bound interaction authority the projection is empty, so the /// generic path cannot serve the prompt and the adapter must say so rather /// than emit an option-less selection. This is also the non-vacuity guard