Engine: filter_binding_diverges' `_ => false` tail is fail-open for the CR 603.4 delayed-trigger hoist

[JacobWoodson]

Summary

filter_binding_diverges (crates/engine/src/game/triggers.rs:9269) ends with a _ => false tail. In this module false means "both legs of the CR 603.4 hoist read this identically, so hoisting is safe" — so an unclassified TargetFilter variant is silently asserted to be reproducible at fire time. That is fail-open in the dangerous direction, and it contradicts the fail-closed policy the surrounding code states and the sibling classifier enforces.

Split out of #7389 rather than fixed there: the fix means adjudicating 54 TargetFilter variants, and flipping the tail alone changes hoist behaviour broadly. Neither belongs in a card fix.

Why this is fail-open

filter_binding_diverges is one of the guards delayed_intervening_if consults before hoisting a delayed triggered ability's intervening-if to fire time (CR 603.4's first half). The two legs of that pair must be the same predicate over the same values:

• returning true → decline the hoist, keep today's resolution-only reading. Costs only CR 603.4's fire-time half.
• returning false → hoist. If the filter actually names a resolution-scoped population, the fire-time leg resolves it against a population the resolver never computed, which can gate the ability off the stack — and for a consumed one-shot, delete it outright (false_gate_consumes_one_shot).

So the cost of a wrong true is a conservative re-check; the cost of a wrong false is a destroyed ability. The tail currently defaults new variants to the destructive answer.

The sibling already fixed exactly this

quantity_ref_binding_diverges (:9079) carries this doc, added in #7389:

EXHAUSTIVE and wildcard-free (matching object_scope_unbound_at_fire_time / quantity_expr_binding_diverges), so a new QuantityRef must be adjudicated here rather than silently defaulting to "cannot diverge" — the earlier _ => false tail rested on exactly that claim and it was false for the resolution-scoped payload-free family below. When in doubt the answer is true: declining costs only the fire-time half of CR 603.4 for that shape, while a wrong false deletes a real ability off the stack.

That is the same tail, the same reasoning, and it was already found to be wrong in practice on the QuantityRef axis. object_scope_unbound_at_fire_time (:8999) and player_scope_unbound_at_fire_time (:9024) are likewise wildcard-free. filter_binding_diverges is the one axis of the four that was not converted.

The gap is structural, not hypothetical: the population axis is reachable from quantity_ref_binding_diverges through the filter-bearing quantity refs (ObjectCount, SacrificedThisTurn, TokensCreatedThisTurn, BattlefieldEntriesThisTurn, …), so a resolution-scoped filter reaches the hoist decision through a classifier that is exhaustive, via one that is not.

There is a second, smaller instance of the same tail on gate_binding_diverges_at_fire_time (:8949), noted by CodeRabbit on #7389. Its blast radius is limited by what ability_condition_to_static_condition can bridge, but it should be settled in the same pass for consistency.

Currently classified

fn filter_binding_diverges(filter: &TargetFilter) -> bool {
    match filter {
        TargetFilter::Typed(tf) => tf.properties.iter().any(|p| matches!(p, FilterProp::Another)),
        TargetFilter::Not { filter } => filter_binding_diverges(filter),
        TargetFilter::And { filters } | TargetFilter::Or { filters } => {
            filters.iter().any(filter_binding_diverges)
        }
        // CR 115.1 + CR 115.10: resolution-scoped anaphora
        TargetFilter::ParentTarget
        | TargetFilter::ParentTargetSlot { .. }
        | TargetFilter::ParentTargetController
        | TargetFilter::ParentTargetOwner
        | TargetFilter::ScopedPlayer => true,
        _ => false,   // <-- 45 or so variants land here unadjudicated
    }
}

Proposed fix

1. Replace _ => false with explicit arms covering all 54 TargetFilter variants, so a new variant fails to compile until someone decides — the property the other three classifiers already have.
2. Adjudicate each against the one question the module asks: does the fire-time QuantityContext (built from the delayed ability's controller, its CR 400.7 source context, and the matched event) bind this population the same way the resolving ability does? Anaphoric / target-relative / per-resolution-local filters are true; live-board and controller-relative populations are false.
3. Apply the module's stated tie-break in the doc comment: when in doubt, true.
4. Settle gate_binding_diverges_at_fire_time's _ => false the same way.
5. Expect some hoists to stop happening. That is the conservative direction and only costs CR 603.4's fire-time half — but it is a behaviour change, so it wants the full suite plus a check that the existing hoist pins (divergent_gate_bindings_decline_the_fire_time_hoist, non_battlefield_presence_gate_declines_the_fire_time_hoist, resolution_scoped_quantity_gate_declines_the_fire_time_hoist) still assert what they intend rather than being silently relaxed.

Not urgent

No current card is known to hit a misclassified filter — this is latent-correctness work, hardening a guard whose failure mode is destructive, not a live bug report. Worth doing before the hoist gets extended to more gate shapes, since each extension widens what reaches this tail.

References

• crates/engine/src/game/triggers.rs:9269 — filter_binding_diverges (the _ => false tail)
• crates/engine/src/game/triggers.rs:9079 — quantity_ref_binding_diverges (the sibling, already exhaustive, with the rationale)
• crates/engine/src/game/triggers.rs:8999 / :9024 — the object- and player-axis classifiers, both wildcard-free
• crates/engine/src/game/triggers.rs:8949 — gate_binding_diverges_at_fire_time (second instance of the same tail)
• Fix Fight for the Throne #7389 — where this was found; CodeRabbit raised the same point as a nitpick there
• CR 603.4 (intervening-if, both halves), CR 115.1 / CR 115.10 (target and player scopes)

[mike-theDude]

Triage analysis

Clarified summary. filter_binding_diverges in crates/engine/src/game/triggers.rs ends in a _ => false tail. In that module false means "both legs of the CR 603.4 intervening-if hoist read this filter identically, so hoisting is safe", so every unclassified TargetFilter variant is silently asserted to be reproducible at fire time. That is fail-open in the destructive direction: a wrong true costs only a conservative re-check, while a wrong false can gate an ability off the stack and, for a consumed one-shot, delete it outright.

Reproduction status. No reproduction exists and none is expected — this is latent-correctness work, not a live user-facing bug. The report is explicit about that, and it is the right call: no currently shipped card is known to reach a misclassified filter.

Rules verification (grepped against docs/MagicCompRules.txt). CR 603.4 is the intervening-if rule, in both its halves (checked when the ability would trigger and again on resolution). CR 115.1 and CR 115.10 cover target and player scopes. The report's citations check out.

Investigation findings. Independently confirmed against the current source: filter_binding_diverges is the only one of the four axis classifiers on this path that still carries a wildcard tail. Its three siblings — quantity_ref_binding_diverges, object_scope_unbound_at_fire_time and player_scope_unbound_at_fire_time — are exhaustive and wildcard-free, and quantity_ref_binding_diverges carries a doc comment recording that its own former _ => false tail was found to be wrong in practice for the resolution-scoped payload-free family. So this is not a hypothetical: the same tail on a sibling axis had a real defect behind it. The reachability argument in the report also holds — the population axis is reachable from the exhaustive quantity_ref_binding_diverges through filter-bearing quantity refs, meaning a resolution-scoped filter can arrive at the hoist decision via an exhaustive classifier that delegates to a non-exhaustive one.

The second, smaller instance of the same tail on gate_binding_diverges_at_fire_time is in scope for the same pass, as the report says.

Likely scope. area:engine, delayed-trigger hoist classification in crates/engine/src/game/triggers.rs. This is a tail-removal refactor, not a new capability: replace the wildcard with explicit arms so a new TargetFilter variant fails to compile until it is adjudicated, then adjudicate the existing variants under the module's stated tie-break (when in doubt, true). Note the scale claim in the body — 54 variants, ~45 landing in the tail — is the report's own count and was not re-derived at triage time; whoever picks this up should re-measure before planning, since TargetFilter has been extended since.

Acceptance criteria.

• filter_binding_diverges and gate_binding_diverges_at_fire_time are exhaustive and wildcard-free, so adding a TargetFilter variant is a compile error until it is classified.
• Each existing variant's classification is justified against the module's one question — does the fire-time QuantityContext bind this population the same way the resolving ability does — with anaphoric/target-relative/resolution-local filters answering true.
• The existing hoist pins (divergent_gate_bindings_decline_the_fire_time_hoist, non_battlefield_presence_gate_declines_the_fire_time_hoist, resolution_scoped_quantity_gate_declines_the_fire_time_hoist) still assert what they intend and were not silently relaxed by the behavior change.
• Any hoists that stop happening are enumerated in the change, since that is a real behavior change even though it is the conservative direction.

Priority rationale. priority:p2-wrong-game-result reflects the failure shape (an ability deleted off the stack is a wrong continuing game result), not current impact — no card is known to hit it today. Not classified as card-data attributable: this is engine-internal guard logic with no Oracle clause behind it.

Related issues. #7389 is where this was found and where the sibling axis was made exhaustive; this was deliberately split out rather than fixed there, which was the right call — adjudicating the full variant set and flipping the tail is not a card fix. Not a duplicate of #7456, which is a different fail-open on the same delayed-trigger path (a hand-set flag rather than a wildcard tail).