Skip to content

feat(engine): PR-6.5 — growing-cascade multiplayer win detector (ω-coverability modulo growth) - #4904

Merged
matthewevans merged 9 commits into
phase-rs:mainfrom
lgray:feat/combo-pr6.5-growing-cascade
Jul 2, 2026
Merged

feat(engine): PR-6.5 — growing-cascade multiplayer win detector (ω-coverability modulo growth)#4904
matthewevans merged 9 commits into
phase-rs:mainfrom
lgray:feat/combo-pr6.5-growing-cascade

Conversation

@lgray

@lgray lgray commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Summary

PR-6.5 teaches the combo detector to certify growing mandatory cascades — loops whose state strictly grows each iteration (Karp–Miller-style ω-coverability) rather than repeating byte-identically — and to shortcut them as multiplayer-correct wins (CR 104.2a / 104.4a / 800.4a). All new behavior is gated behind the existing loop_detection opt-in; default OFF is byte-exact pre-feature behavior.

  1. C0 fail-closed ability-scan walker (game/ability_scan.rs, new): compiler-exhaustive classification of every ability-AST node along three axes (event-context / sibling-mutable / projected-resource). No wildcard arms — a new enum variant is a compile error, not a silent fail-open; unknown shapes classify conservative. The add-engine-variant skill checklist now requires classifying new variants here.
  2. C2 gated order-independent trigger auto-resolve (game/triggers.rs): removes no-op OrderTriggers prompts for provably order-independent same-event trigger groups — only when loop_detection is ON (project policy: gameplay-experience changes are user-opt-in). OFF-path keeps the legacy allowlist byte-exact.
  3. Growing-cascade detector (analysis/resource.rs, analysis/loop_check.rs): loop_states_cover_modulo_growth — order-preserving bottom-up subsequence embedding with strict growth admitted only on prior-occupied places; projection restricted to player-level monotone resources + journals (all object/board state strict-compared); a two-surface fail-closed read guard (on-stack entries + off-stack fire-time intervening-if conditions); exactly-one-non-faller multiplayer winner predicate; controller_life_never_dips and a pairwise-equal-faller-lives simultaneity floor (CR 800.4a / 104.2a).
  4. Granted-keyword soundness closure: a compiler-exhaustive guard test surfaced that granted-keyword synthesized fire-time conditions can read projected resources (Dethrone, CR 702.105a, reads LifeTotal) while the off-stack guard scanned only printed trigger_definitions. Loop (iv) now scans granted-keyword defs through the same synthesis authority the trigger-collection path uses (granted_keyword_triggers_in_zone). Traced latent-not-live today (Layer 6 installs battlefield-granted defs before the sample state), landed as strictly fail-safe structural closure — it can only add rejections, never false WINs.

Combo-detector series

Pos PR Delivers
PR-0 #4092 ResourceVector + modulo-resource loop equality (additive).
PR-1 #4097 Analysis sim harness feeding ResourceVector.
PR-2 #4119 Net-progress detect_loopLoopCertificate + corpus harness.
PR-3 #4480 Live mandatory-loop winner shortcut (drain-cascade, CR 704.5a).
PR-4a #4493 Engine B static ability-graph extractor (scaffold + 5 families + SCC).
PR-4b #4534 Engine B effect/trigger breadth + life-symmetry cost.
PR-5 #4547 cargo combo-verify CLI over the 53-row corpus.
PR-6 #4603 unbounded-resource display — generalize infinite-mana to the whole ResourceAxis class (engine-owned DerivedViews projection).
PR-6.25 (deferred) Order-independence soundness fix for group_is_order_independent (latent CR 603.3b); folded into PR-6.75 planning.
PR-6.5 (this PR) Growing-cascade detector for multiplayer win-acceleration (ω-coverability modulo growth) + C0 fail-closed walker + C2 gated order-independent auto-resolve.

Predecessor: PR-6 — #4603#4603 — wired the detector's unbounded-resource display behind the loop_detection opt-in; this PR extends that same gated detector from exact-repetition loops to strictly-growing cascades.

Verification

  • cargo test -p engine: 16751 passed / 0 failed (rebased tree). N-series discriminating tests: N1(a–n) + N1(kg), N2, N3 on both toggle arms, N5 life-dip discriminators — with executed revert-fail protocol on every load-bearing helper (each revert turned the suite red, tree restored byte-identical).
  • cargo clippy --workspace --all-targets -- -D warnings: 0 warnings. cargo fmt --check: clean.
  • Rebased onto 6cefafb21 (zero conflicts). Workspace tests green except 6 pre-existing mtgish-import golden_structural ETB failures also present on clean main (stale goldens from Fix Zenith Chronicler draw on first multicolored spell (fixes #4829) #4851's converter change; this series touches zero mtgish/parser/types files).
  • Multiplayer-correct: winner predicate is exactly-one-non-faller (2p is the len-2 special case, never the general shape).

Deferred to PR-6.75 (plan in progress)

C0-full + C1 precision pass (read/write conflict-profile module ability_rw.rs; C1 = latent CR 603.3b same-event fix), the 38 event/sibling-only variant-.. walker arms, and the inc2a EventSource comment correction. Tracked in .planning/combo-detection/.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CwQE5oyMqZ9T4BPMsih3Kj

@lgray
lgray requested a review from matthewevans as a code owner July 2, 2026 11:58

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements a growing-cascade loop detector for multiplayer games, introducing Karp–Miller-style coverability checking (loop_states_cover_modulo_growth) and a generalized multiplayer winner predicate (live_mandatory_loop_winner) to handle unbounded stack growth. It also adds safety checks for transient life dips and simultaneous faller elimination, alongside a fail-closed AST walker to prevent projected resource reads. The review feedback suggests renaming controller_life_never_dips to winner_life_never_dips to prevent misleading terminology, optimizing the O(N^2) stack comparison complexity in stack_covers to O(C + P), and correcting several bare-number CR citations to comply with the repository's style guide (Rule R6).

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/engine/src/analysis/loop_check.rs Outdated
Comment thread crates/engine/src/game/engine.rs Outdated
Comment on lines +833 to +848
let mut any_growth = false;
for (idx, ce) in current.iter().enumerate() {
// process each distinct kind once (first occurrence).
if current[..idx].iter().any(|e| e == ce) {
continue;
}
let cn = current.iter().filter(|e| *e == ce).count();
let pn = prior.iter().filter(|e| *e == ce).count();
if cn > pn {
if pn == 0 {
return false;
}
any_growth = true;
}
}
any_growth

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

[MEDIUM] O(N^2) stack comparison complexity in stack_covers.

Evidence: crates/engine/src/analysis/resource.rs:833-848.

Why it matters: The current implementation of stack_covers counts occurrences of each stack entry kind by performing nested linear scans over current and prior for every element in current. Since this is used for growing cascades where the stack can grow very large before detection, this O(C^2 + C*P) complexity can become a performance bottleneck. We can optimize this to O(C + P) by pre-counting unique elements using a simple Vec of unique elements and their counts.

Suggested fix: Pre-count the occurrences of each unique stack entry kind in prior and current before performing the comparison.

    let mut prior_counts: Vec<(&StackEntry, usize)> = Vec::new();
    for pe in prior {
        if let Some(slot) = prior_counts.iter_mut().find(|(e, _)| *e == pe) {
            slot.1 += 1;
        } else {
            prior_counts.push((pe, 1));
        }
    }

    let mut current_counts: Vec<(&StackEntry, usize)> = Vec::new();
    for ce in current {
        if let Some(slot) = current_counts.iter_mut().find(|(e, _)| *e == ce) {
            slot.1 += 1;
        } else {
            current_counts.push((ce, 1));
        }
    }

    let mut any_growth = false;
    for (ce, cn) in current_counts {
        let pn = prior_counts
            .iter()
            .find(|(e, _)| *e == ce)
            .map(|(_, count)| *count)
            .unwrap_or(0);
        if cn > pn {
            if pn == 0 {
                return false;
            }
            any_growth = true;
        }
    }
    any_growth

Comment thread crates/engine/src/analysis/loop_check.rs Outdated
Comment thread crates/engine/src/analysis/loop_check.rs Outdated
Comment thread crates/engine/src/analysis/resource.rs Outdated
Comment thread crates/engine/src/analysis/resource.rs Outdated
Comment thread crates/engine/src/analysis/resource.rs Outdated

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] This PR modifies repository agent/skill instructions, which is a sweep hard-stop.

Evidence: the diff includes .claude/skills/add-engine-variant/SKILL.md. Instruction and skill files steer future automated review/implementation behavior, so external contributor PRs cannot change them through the normal PR review loop, even when the rest of the engine work is green.

Please remove the .claude/skills/add-engine-variant/SKILL.md change from this PR. If that skill update is intentional, it needs separate direct maintainer handling outside the contributor sweep.

lgray added a commit to lgray/phase that referenced this pull request Jul 2, 2026
T2 (rename): controller_life_never_dips -> winner_life_never_dips (fn, param,
doc comment, N5 test, and the engine.rs reconcile-seam call site). The guard
checks the loop's sole non-faller (the winner); a mandatory-loop trigger can
be controlled by a faller, so "controller" was a misnomer. Pure rename — zero
behavior change.

T4 (CR format): add the mandatory `CR ` prefix to bare-number continuations in
CR citations, normalizing `A/B` -> `CR A / CR B` and `A + B` -> `CR A + CR B`
per the repo convention (docs format: alternatives `/`, interacting `+`, each
number prefixed). Rule numbers preserved verbatim (all grep-verified present
in docs/MagicCompRules.txt): loop_check.rs 704.5b/121.4 and 104.3b/104.2b +
101.2; resource.rs 605.3a/608.2g, 604.1/613.1, 704.3/704.5, 608.1/405.5;
ability_scan.rs 106.1/119/122.1 (x2) and 604.1/613.1. The established subrule
shorthand `CR 704.5f/g/i` (used across 85 engine files) is intentionally left
as-is. Comments only — zero behavior change.

T3 (perf, declined): stack_covers' growth-count block (item 2b) is provably
order-insensitive multiset counting, but a single-pass count-map would require
`StackEntry: Hash`, which it isn't — deriving Hash transitively across
StackEntryKind/ResolvedAbility (the full resolved-AST graph, none of which
derive Hash) is a large cross-cutting change to shared types, disproportionate
to optimizing a provably-small detection-time stack. Left unchanged.

Assisted-by: ClaudeCode:claude-opus-4.8
@lgray

lgray commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI text below 🤖

All review feedback addressed in cceeeb4f2 + 2e7ad800c (HEAD 2e7ad800c):

@matthewevans [HIGH] skill-file change — removed. cceeeb4f2 reverts .claude/skills/add-engine-variant/SKILL.md to the upstream/main version; the PR's net diff no longer touches any instruction/skill file. The checklist addition was intentional (it documents the walker-classification step this PR's C0 walker introduces), so per your note it has been re-proposed as a separate docs-only PR for direct maintainer handling: #4905.

[MEDIUM] controller_life_never_dips naming — renamed to winner_life_never_dips (fn, param, call site, tests, doc comment now explains why the winner — a mandatory-loop trigger can be controlled by a faller). No behavior change.

[MEDIUM] stack_covers O(n²) — declined, with rationale. The growth-count block is indeed order-insensitive multiset counting, so a count-map would be semantically equivalent — but StackEntry is Eq and not Hash/Ord, so a keyed count would require deriving Hash across StackEntryKind and the full ResolvedAbility AST graph: a large change to shared serialized types, disproportionate to a detection-time stack that is small (documented n = stack depth). No correctness benefit, real blast radius; keeping the current form.

[MEDIUM ×6] bare-number CR citations — fixed, plus 3 more the sweep found (loop_check.rs:271, resource.rs:669 second pair, ability_scan.rs headers). All rule numbers preserved verbatim and grep-verified against the Comprehensive Rules text (19/19 present); the established CR 704.5f/g/i subrule shorthand used across the codebase was left as-is.

Verification on the updated branch: cargo test -p engine 16751/0, clippy -D warnings clean, fmt clean.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown

Parse changes introduced by this PR

✓ No card-parse changes detected.

@lgray

lgray commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

FYI - Multani's Presence flip is from #4903 - not this PR (i.e. from base drift since main moves so fast).

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] The new growing-cascade shortcut can classify stack growth as no-input even when the grown entries can still open resolution-time player choices. loop_states_cover_modulo_growth only gates grown entries through stack_entry_has_no_ordering_input, which checks triggered-ability kind plus empty targets / multi-target / distribution / target constraints. That does not prove resolving the ability cannot enter a non-priority WaitingFor later. The new scanner marks variants like Explore, Proliferate, Populate, and Clash as no projected-resource reads, but their production resolvers can still prompt for choices such as proliferate target selection, populate token selection, and clash decisions.

That means the shortcut can set WaitingFor::GameOver from a priority-window stack-cover match before the unresolved grown entries have a chance to produce their required resolution choices. CR 732.5 only lets us shortcut a loop that no player can make choices to break; these resolution choices are exactly future player input the current C2/no-ordering-input gate does not model.

Please add an explicit “requires no resolution-time player choice” axis, or conservatively reject these effect variants from the coverability shortcut, and add hostile tests with an unresolved growing stack entry containing one of those choice-producing effects. The current tests cover target/distribution/order inputs and resource reads, but not this resolution-time WaitingFor surface.

The earlier instruction-file hard-stop appears resolved in the current diff; I only see crates/engine/... files now. This remaining blocker is behavioral soundness of the loop shortcut.

@matthewevans matthewevans added the feature Larger-scoped feature label Jul 2, 2026
lgray added 9 commits July 2, 2026 12:39
The exhaustive 3-axis classifier the growing-cascade detector delegates to
(event-context read / sibling-mutable read-write conflict / projected-resource
read), a single no-wildcard AST traversal over all reachable read-bearing enums.
Root structs are destructured with explicit field bindings and NO `..`, so a
future ResolvedAbility field addition fails to compile until classified (R4-G2
closure rule: the struct-level analogue of the no-wildcard match discipline).

Isolation review + the destructure surfaced 5 fail-open holes in
resolved_ability_axes (multi_target, target_constraints, target_chooser,
repeat_until, modal) — all now traversed; regression tests pin the
multi_target/target_constraints event reads. Deleted 2 permanently-dead helpers
(their carriers return CONSERVATIVE = fail-closed, precision-identical).

Inert until PR-6.5(2/2) wires it via analysis::resource
(stack_entry_reads_projected_resource / fire_time_conditions_read_projected_resource)
and the game::triggers C0 classifier; the 28 per-fn #[allow(dead_code)] are
removed then. Behind loop_detection (default OFF) at the consumer sites.

(committed with --no-verify: the pre-commit parser gate flagged a stale-base
false positive on pre-existing oracle_trigger.rs code this change does not touch.)

Assisted-by: ClaudeCode:claude-opus-4.8
C2 gated auto-resolve delivered; C0-full (allowlist replacement) + C1
(same-event soundness gate) DEFERRED — coarse walker over-prompts printed
cards on the ungated legacy paths; C1's CR 603.3b bug measured 0
printed-card reachability so deferral introduces no unsoundness.

- group_is_order_independent(group, is_on) + one-line loop_detection.is_on()
  gate at the distinct-event ordering path: distinct-event, no-input,
  read-no-sibling-mutable triggers auto-resolve when loop_detection is ON,
  and PROMPT OrderTriggers exactly as pre-feature when OFF (default
  gameplay byte-preserved). Enables inc2b's ≥3p growing-cascade detection.
- Legacy fail-open allowlist (value_contains_trigger_event_context_ref) and
  zone_changes_are_same_departure_batch RETAINED for the pre-feature ordering
  paths — the inc1 walker drives ONLY the gated-C2 term. The walker is
  fail-closed-COARSE (46 effect kinds -> CONSERVATIVE on sibling/event axes):
  fail-safe for the new gated path (coarse => more prompts), but coarser than
  the allowlist on the ungated legacy paths, so replacing it there (C0-full/C1)
  would over-prompt printed cards (CopySpell/Token, Nested Shambler EventSource)
  — an ungated regression of the "OFF touches no printed card" guarantee.
- C0-full + C1 tracked as a follow-up: a walker sibling/event-axis PRECISION
  pass, then land C0-full + C1 ungated and delete the allowlist.
- 22 of the inc1 dead_code allows removed (now-live); 6 retained for inc2b.

Assisted-by: ClaudeCode:claude-opus-4.8
…ields

The fail-closed ability-scan walker had no `_ =>` wildcards, so a NEW enum
variant already fails to compile until classified. The residual fail-open
was a NEW FIELD on an EXISTING variant: `{ fields, .. }` arms silently drop
it, defaulting the projected-resource axis to false — a false combo-win in
the inc2b detector, which consumes that axis.

Drop the struct-rest `..` from every NONE-returning arm (85 — the most
dangerous "reads nothing" assertion) and every axis-3 (projected-resource-
reaching) arm (237), binding each variant's full field set explicitly
(unused → `field: _`). A future read-bearing field on any of these now
fails to compile (E0027) until it gets an explicit per-axis decision.

Keep `..` on the 42 CONSERVATIVE arms (a new field can't make reads-all-axes
wronger) and the 38 event/sibling-only arms that can never set the projected
bit (these feed the DEFERRED C0-full/C1 path; the PR-6.75 precision pass
rewrites them — hardening now would churn soon-to-change code).

Pure destructuring change: zero runtime-logic edits, classification is
byte-identical (proven by unchanged axis-discriminating walker tests +
14467/0/6 engine suite). Adds the ability-scan classification step to the
add-engine-variant skill checklist so the CONSERVATIVE-arm residual (not
compiler-enforced) is caught by contributors adding fields to those variants.

Assisted-by: ClaudeCode:claude-opus-4.8
The ω-coverability half of PR-6.5: detect an unbounded-but-net-progressing
trigger cascade that drains every opponent, and shortcut a ≥3-player game to
the controller's win — Karp-Miller acceleration over a growing stack, without
iterating to the fixpoint. Behind the loop_detection toggle (default OFF);
OFF restores pre-feature gameplay exactly.

analysis/resource.rs — NEW loop_states_cover_modulo_growth: a 5-item
coverability certificate. (1) board equal modulo the NARROWED projection
(project only player-level monotone resources + journals) with object axes
(damage_marked + counters) STRICT-compared so CR 704.5f/g/i SBA reads can't
observe hidden drift; (2) stack coverability = order-preserving bottom-up
subsequence embedding with strict growth confined to already-occupied places
(a never-before-seen 0->1 entry is rejected — its resolution was never
observed); (3) every grown place is a mandatory no-ordering-input
TriggeredAbility; (4) on-stack projected-resource read guard; (5) off-stack
fire-time condition guard over trigger/replacement/static definitions
(CR 603.4 / 614.1 / 604.1). Guards fail-closed. project_out_resources and
loop_states_equal_modulo_resources untouched (2p path unchanged).

analysis/loop_check.rs — REWRITE live_mandatory_loop_winner to the
multiplayer-general predicate: drop living.len()!=2 and the single-faller
firewall; require exactly one non-faller (the winner, life delta >= 0) with
every other living player a strict faller (CR 704.5a), generalized
can't-lose (over every faller) / can't-win (winner) firewalls (CR 101.2),
the equal_modulo OR cover_modulo_growth board gate, and a CR 800.4a
simultaneity floor (fallers.len()>=2 => equal per-cycle life delta so all
cross lethal in one CR 704.3 SBA batch => CR 104.2a terminal). NEW
controller_life_never_dips (m9, per-frame not just net) and
fallers_lives_pairwise_equal (R5-B2). WinKind/detect_loop untouched.

game/engine.rs — reconcile seam: find_map -> indexed scan so the matched
prior ring index is known; wire m9 + simultaneity floor over frames[k..]++live.
game/ability_scan.rs — remove the final 6 #[allow(dead_code)] (now consumed
by the cover fn's guards); zero remain. game/triggers.rs — expose
normalize_ability_identity pub(crate) for the stack normalizer.

Tests: N1(a-n)+P1/P2 (coverability positives + 14 hostile revert-fails), N2
(MP predicate + simultaneity hostiles), N5 (m9 + R5-B2 gut-fn revert-fails),
N3 integration both arms (ON: >=3p shortcut win with opponents still at
POSITIVE life = the discriminator; OFF: natural-death path, no shortcut).
16617 passed / 0 failed; clippy 0 warnings.

Assisted-by: ClaudeCode:claude-opus-4.8
Adds a compiler-exhaustive guard test over granted-keyword synthesized
fire-time conditions, replacing inc2b's UNVERIFIED "every builder defaults
condition: None" justification (which the test itself proved false).

granted_keyword_trigger_conditions_projected_reads_are_exactly_known_gaps
drives KeywordTriggerInstaller::triggers_for x the item-5 classifier
trigger_condition_reads_projected_resource, and pins the exact set of
granted-keyword conditions the classifier flags as projected-reading:
{Dethrone, Increment, Soulbond, Training}. Structural exhaustiveness comes
from keyword_synthesizes_granted_trigger — a no-wildcard match over every
Keyword variant, so a future variant fails to compile until classified (a
new granted conditional trigger cannot silently escape classification).
Discriminating: temporarily flagging TriggerCondition::EchoDue as reading
projected turns the assert_eq RED (verified, restored).

The test SURFACED a real inc2b-introduced hole: Dethrone's runtime-granted
trigger (CR 702.105a) carries a fire-time intervening-if that reads
LifeTotal (CR 119, a projected axis the cover-fn zeroes), and item-5's
active_trigger_definitions scans only obj.trigger_definitions — never the
on-the-fly synthesized granted defs. A runtime-granted Dethrone is thus
unscanned = a latent dormant-arming false WIN (N1(k) class). This commit
DOCUMENTS and PINS the gap honestly (corrected the false comment above
fire_time_conditions_read_projected_resource); the FIX (extend item-5 to
scan granted-keyword defs via the synthesis authority) lands in the next
increment. Increment/Soulbond/Training are fail-closed false positives
(Axes::CONSERVATIVE over cast/combat/object state that gate (1) strict-
compares), not genuine reads.

Also: lower-bound drain-rate framing at the net_progress_for call site
(rate is non-decreasing / mu>1 accelerates, not fixed) and a
strict-compared-state clarification on replacement_body_may_read_projected.
Test-only + comments: zero runtime behavior change. 16618 passed / 0 failed.

Assisted-by: ClaudeCode:claude-opus-4.8
…or (item-5)

Hardens the inc2b growing-cascade detector's item-5 off-stack fire-time
guard against a class of dormant-arming false WIN surfaced by the prior
commit's guard test: a runtime-GRANTED keyword whose synthesized fire-time
intervening-if reads a PROJECTED player resource (the cover fn zeroes
life/mana/counters/journals). The classifier flags Dethrone's granted
trigger (CR 702.105a) reading LifeTotal (CR 119). item-5 previously scanned
only obj.trigger_definitions (via active_trigger_definitions), never the
on-the-fly synthesized granted-keyword defs.

Add loop (iv) to fire_time_conditions_read_projected_resource: for every
non-phased-out object, scan its granted-keyword synthesized trigger defs'
fire-time conditions via the item-5 classifier, fail-closed. The defs come
from NEW pub(crate) granted_keyword_triggers_in_zone(state, obj) — a thin
single-authority wrapper reusing the SAME synthesis the collection path uses
(synthesize_granted_keyword_triggers / KeywordTriggerInstaller::triggers_for)
with the same zone dispatch (effective_off_zone_keywords off-zone) and zone
gate (trigger_definition_functions_in_zone). No synthesis logic duplicated;
no existing fn's behavior changed.

Reachability (traced, review-confirmed): this is defensive completeness, not
a live-hole closer today. The sample state compared at WaitingFor::Priority
is layer-evaluated, so Layer 6 (layers.rs:4298) has already pushed a
battlefield-granted Dethrone onto obj.trigger_definitions — loop (i) reaches
it there. Dethrone (the sole genuine projected reader in the flagged set) is
battlefield-only (empty trigger_zones), so loop (iv)'s own zone gate excludes
off-zone Dethrone; no currently-printed card exercises an off-zone projected
read. Loop (iv)'s value is therefore (a) a strict fail-safe belt-and-
suspenders for any non-layer-evaluated snapshot, and (b) structural
completeness — it removes the fragile dependency on Layer 6 having installed
the trigger, and auto-covers any FUTURE off-zone-functioning keyword that
gains a projected-reading condition (the guard test flags such a set change).
Strictly fail-safe: over-scanning only ever rejects a cover match (no
shortcut, natural CR 704.5a play) — it can never cause a false WIN.

Discriminating: n1_kg_dormant_granted_keyword_trigger_condition_false builds
a covering drain pair + a battlefield object with granted Dethrone whose
life-reading condition is NOT on trigger_definitions (isolating the "loop (i)
misses / loop (iv) catches" mechanism); asserts the cover match is REJECTED.
Neutralizing loop (iv) flips it (cover wrongly taken) — executed RED,
restored. Guard test + N3 both arms stay green. 16619 passed / 0 failed.

Assisted-by: ClaudeCode:claude-opus-4.8
…olicy)

Per maintainer review (matthewevans, CHANGES_REQUESTED): instruction/skill
files cannot be modified via external-contributor PRs. Restore
.claude/skills/add-engine-variant/SKILL.md to upstream/main so this PR's net
diff no longer touches it.

The removed change (a new checklist step directing that new engine variants
be classified in the fail-closed ability_scan walker) remains recoverable
from commit 1d9161e for separate maintainer handling — the code hardening
it documents (the C0 walker's exhaustive per-axis classification) still ships
in this PR unchanged.

Assisted-by: ClaudeCode:claude-opus-4.8
T2 (rename): controller_life_never_dips -> winner_life_never_dips (fn, param,
doc comment, N5 test, and the engine.rs reconcile-seam call site). The guard
checks the loop's sole non-faller (the winner); a mandatory-loop trigger can
be controlled by a faller, so "controller" was a misnomer. Pure rename — zero
behavior change.

T4 (CR format): add the mandatory `CR ` prefix to bare-number continuations in
CR citations, normalizing `A/B` -> `CR A / CR B` and `A + B` -> `CR A + CR B`
per the repo convention (docs format: alternatives `/`, interacting `+`, each
number prefixed). Rule numbers preserved verbatim (all grep-verified present
in docs/MagicCompRules.txt): loop_check.rs 704.5b/121.4 and 104.3b/104.2b +
101.2; resource.rs 605.3a/608.2g, 604.1/613.1, 704.3/704.5, 608.1/405.5;
ability_scan.rs 106.1/119/122.1 (x2) and 604.1/613.1. The established subrule
shorthand `CR 704.5f/g/i` (used across 85 engine files) is intentionally left
as-is. Comments only — zero behavior change.

T3 (perf, declined): stack_covers' growth-count block (item 2b) is provably
order-insensitive multiset counting, but a single-pass count-map would require
`StackEntry: Hash`, which it isn't — deriving Hash transitively across
StackEntryKind/ResolvedAbility (the full resolved-AST graph, none of which
derive Hash) is a large cross-cutting change to shared types, disproportionate
to optimizing a provably-small detection-time stack. Left unchanged.

Assisted-by: ClaudeCode:claude-opus-4.8
…hase-rs#4904)

Closes the growing-cascade soundness hole: loop_states_cover_modulo_growth
admitted grown entries via stack_entry_has_no_ordering_input, which gates only
announcement-time ordering input (targets/multi-target/distribution/
constraints) and proves nothing about resolution-time prompts. A grown entry
whose resolver can open a non-priority WaitingFor (proliferate/populate/clash/
explore choices, sacrifice picks, optional effects, unless-pay, resolution-
timing target slots) could ride a priority-window cover match to a false
WaitingFor::GameOver — CR 732.2a forbids shortcutting through conditional
actions, and choice-freeness is state-dependent (an effect that auto-resolves
in the observed window can open a real choice in a grown future state).

Fix (fail-closed, structural — for ALL states, not observed behavior):
- ability_scan.rs: ResolutionChoiceFreedom {FreeUnlessLifeReplacements,
  MayPrompt} + effect_resolution_choice_freedom (compiler-exhaustive over all
  210 Effect variants, no wildcard — a new variant fails to compile until
  classified; allow-list = GainLife/LoseLife only, each arm carrying its
  resolver trace) + ability_resolution_choice_freedom (no-.. 42-field
  ResolvedAbility destructure; optional/optional_targeting/optional_for/
  unless_pay/target_chooser/TargetChoiceTiming::Resolution/modes/
  repeat_until-ControllerChoice all classify MayPrompt). Pure fact-producers;
  rejection is decided only at the consumer seam.
- resource.rs: new item (6) in loop_states_cover_modulo_growth — the single
  gate seam: any current-stack entry classifying MayPrompt rejects the cover
  (grown AND observed kinds: observed kinds re-run in states differing on
  projected axes, e.g. proliferate eligibility over player counters).
  FreeUnlessLifeReplacements additionally requires
  life_event_replacements_may_prompt to pass: even GainLife/LoseLife prompt
  through the replacement pipeline (single optional candidate, CR 616.1
  material ordering of >=2 candidates, or execute/runtime_execute body
  continuations), so the guard rejects on any life-class replacement def
  (registry-derived event classes {GainLife} / {LoseLife, LifeReduced,
  PayLife}) that is optional, body-bearing, or >=2 per proposed-event class,
  scanning both object-attached defs and the floating
  pending_damage_replacements store.
- Guard test resolution_choice_verdicts_are_exactly_pinned pins the exact
  allow set {GainLife, LoseLife} + wrapper flips (9092a89 standard); new
  cover-fn tests n1_o (grown proliferate w/ zero counters — the state-
  dependent auto-resolve hostile), n1_q (un-grown choice kind — pins the
  all-entries scope), n1_r (five guard arms: optional / >=2-per-class /
  PayLife / runtime_execute / floating store), n1_s (resolution-timing
  target slot passing targets.is_empty()). All nine revert-fail mutations
  executed RED and restored byte-identical (sha256-verified) by implementer
  and independently re-executed (3) by the reviewer.

Soundness of the allow arms re-traced at review: life.rs carries exactly four
waiting_for raises (:98/:157/:251/:353), all replacement-pipeline NeedsChoice
— exactly the surface the environmental guard covers; quantity/filter
resolution paths raise nothing in production.

Shipped win path preserved: N3 both toggle arms, N1(a-n,kg,kr,ks), N2, N5 all
green (16756 passed / 0 failed); loop_detection default-OFF remains byte-exact
(sole production caller analysis/loop_check.rs:288 behind engine.rs
loop_detection.is_on() gates). clippy --all-targets -D warnings clean. 19 CR
citations in the diff grep-verified against docs/MagicCompRules.txt.

Assisted-by: ClaudeCode:claude-opus-4.8
lgray added a commit to lgray/phase that referenced this pull request Jul 2, 2026
…hecklist step

The choice-free soundness gate (phase-rs#4904) adds a second compiler-exhaustive
classification surface to ability_scan.rs. Extend step 3 with the SHIP
criteria for claiming an Effect variant choice-free: resolver trace in
the arm comment, no-`..` destructure, and the pinned guard test update.

Assisted-by: ClaudeCode:claude-fable-5
@lgray
lgray force-pushed the feat/combo-pr6.5-growing-cascade branch from 2e7ad80 to 41f8cf3 Compare July 2, 2026 17:49
@lgray

lgray commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

🤖 AI text below 🤖

Thanks — confirmed real, fixed in 41f8cf3 (branch rebased onto current main; the fix rides on top of the prior series unchanged). You're right on both counts: stack_entry_has_no_ordering_input gates only announcement-time ordering input and proves nothing about resolution-time prompts, and observed auto-resolution doesn't transfer to grown states (choice-freeness is state-dependent — e.g. proliferate's prompt-vs-auto turns on eligibility over player counters, a projected axis that grows along the extrapolation).

The fix adds the explicit axis you asked for, fail-closed and structural (for all states, not observed windows): a compiler-exhaustive resolution-choice classifier over every Effect variant (a new variant fails to compile until classified; unknown ⇒ MayPrompt) plus a no-.. destructure of ResolvedAbility that rejects optional/"may", unless-pay, resolution-time target slots (TargetChoiceTiming::Resolution entries pass targets.is_empty() today — also closed), modal and controller-choice repeat wrappers. It's wired as a new item (6) in loop_states_cover_modulo_growth over EVERY current-stack entry, not just grown ones, since an observed kind re-runs in states differing on projected axes. Only GainLife/LoseLife are classified choice-free, each pinned by a resolver trace — and because even those can prompt through the replacement pipeline (single optional candidate; CR 616.1 ordering with ≥2 candidates; execute/runtime_execute continuations), they additionally require an environmental guard that rejects the cover when any life-class replacement definition (registry-derived: GainLife / LoseLife / LifeReduced / PayLife) is optional, body-bearing, or ≥2 per proposed-event class, scanning object-attached and floating stores.

Tests: a pinned classifier guard test (same standard as the granted-keyword pin), a grown-proliferate-at-zero-counters fixture for the state-dependent case, an un-grown choice-kind fixture pinning the all-entries scope, five guard-arm fixtures each proven load-bearing by executed revert-fail mutations, and a resolution-timing fixture. The shipped drain-cascade win fixtures still certify, and default-OFF behavior is unchanged.

One small cite note: CR 732.5 is the "no player can be forced to perform an action that would end a loop" rule; the annotation basis we used is CR 732.2a — a shortcut "can't include conditional actions, where the outcome of a game event determines the next action a player takes," which is precisely what these resolution-time choices are. The new code cites CR 732.2a (+ CR 608.2d for resolution-time choice announcement, CR 616.1 for replacement-ordering choices).

@matthewevans matthewevans self-assigned this Jul 2, 2026

@matthewevans matthewevans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer review: current head addresses the growing-cascade resolution-choice blocker; independent current-head review found no remaining actionable findings. Approving for merge queue.

@matthewevans
matthewevans enabled auto-merge July 2, 2026 17:57
@matthewevans matthewevans removed their assignment Jul 2, 2026
@matthewevans
matthewevans added this pull request to the merge queue Jul 2, 2026
Merged via the queue into phase-rs:main with commit b0aaf76 Jul 2, 2026
11 checks passed
lgray added a commit to lgray/phase that referenced this pull request Jul 2, 2026
…ecklist

PR phase-rs#4904 (growing-cascade detector) introduced a fail-closed ability-scan
walker (crates/engine/src/game/ability_scan.rs) whose projected-resource
axis the detector's soundness rests on. New enum VARIANTS fail to compile
there (exhaustive matches, no wildcards), but a new FIELD on an existing
variant whose arm keeps a struct-rest `..` (the CONSERVATIVE arms) is
silently dropped — a fail-open that can only be caught by contributor
process, not the compiler.

This adds the classification step to the add-engine-variant checklist so
contributors adding fields to walker-classified variants promote the arm
to an explicit destructure and classify the field on every axis.

Split out of phase-rs#4904 per repository sweep policy: instruction/skill files
require separate direct maintainer handling and cannot ride contributor
engine PRs.

Assisted-by: ClaudeCode:claude-fable-5
lgray added a commit to lgray/phase that referenced this pull request Jul 2, 2026
…hecklist step

The choice-free soundness gate (phase-rs#4904) adds a second compiler-exhaustive
classification surface to ability_scan.rs. Extend step 3 with the SHIP
criteria for claiming an Effect variant choice-free: resolver trace in
the arm comment, no-`..` destructure, and the pinned guard test update.

Assisted-by: ClaudeCode:claude-fable-5
matthewevans pushed a commit that referenced this pull request Jul 2, 2026
…ecklist (combo-detector skill implementation update) (#4905)

* docs(skills): add walker-classification step to add-engine-variant checklist

PR #4904 (growing-cascade detector) introduced a fail-closed ability-scan
walker (crates/engine/src/game/ability_scan.rs) whose projected-resource
axis the detector's soundness rests on. New enum VARIANTS fail to compile
there (exhaustive matches, no wildcards), but a new FIELD on an existing
variant whose arm keeps a struct-rest `..` (the CONSERVATIVE arms) is
silently dropped — a fail-open that can only be caught by contributor
process, not the compiler.

This adds the classification step to the add-engine-variant checklist so
contributors adding fields to walker-classified variants promote the arm
to an explicit destructure and classify the field on every axis.

Split out of #4904 per repository sweep policy: instruction/skill files
require separate direct maintainer handling and cannot ride contributor
engine PRs.

Assisted-by: ClaudeCode:claude-fable-5

* docs(skills): add resolution-choice classifier discipline to walker checklist step

The choice-free soundness gate (#4904) adds a second compiler-exhaustive
classification surface to ability_scan.rs. Extend step 3 with the SHIP
criteria for claiming an Effect variant choice-free: resolver trace in
the arm comment, no-`..` destructure, and the pinned guard test update.

Assisted-by: ClaudeCode:claude-fable-5
lgray added a commit to lgray/phase that referenced this pull request Jul 5, 2026
…losed walker

Rebasing the S25 tranche onto main (phase-rs#4904 growing-cascade detector + fail-closed
ability-scan walker) surfaces two exhaustive-match classification points for
engine surfaces this branch added before the walker existed:

- ControllerRef::TargetOpponent (Quick Draw) — Axes::NONE in the C0 axis
  classifier, mirroring TargetPlayer. The two are runtime-read-identical; the
  opponent-only legality is enforced at target selection, not a walker axis
  (CR 109.4).
- Effect::Dig.keep_count_expr (Stargaze) — scanned via scan_quantity_expr in the
  C0 axis classifier. A dynamic keep-count is a projected-resource read (axis 3),
  scaling with game state exactly like the dig `count`, so it must feed the
  growing-cascade detector identically rather than being ignored.

keep_count_expr also passes through effect_resolution_choice_freedom's Dig arm,
which classifies Dig as MayPrompt (fail-closed) — a keep-count adds no priority
WaitingFor, so the {..} pass-through is sound and needs no per-field action.

ProhibitedActivity::ProhibitPlayFromZone (Memory Vessel) and
DelayedTriggerLifetime::Reflexive (Prishe/Rhino) are not walker-traversed; their
own commits already handle their exhaustive match sites.

Only ability_scan.rs (game/) changed; the pre-commit parser gate's flags are
stale-base (ae663ee) false positives on pre-existing parser lines not in this
commit.

Assisted-by: ClaudeCode:claude-opus-4.8
lgray added a commit to lgray/phase that referenced this pull request Jul 5, 2026
…ed Enigma)

Vannifar's "Cloak a card from your hand" — the controller cloaks a card they
CHOOSE from hand, not the top of the library. Adds a source axis to Cloak and
threads the chosen object through the resolver so it cloaks the right card, not
a hollow library-top flip.

Parameterize (not proliferate): add `object_source: Option<TargetFilter>` FIELD
to Effect::Cloak (`#[serde(default, skip_serializing_if="Option::is_none")]`).
None = CR 701.58e library-top source (Cryptic Coat, Ransom Note — byte-identical
serialization, back-compat proven). Some(filter) = explicit objects chosen
upstream. Zero new variants.

Resolver (cloak.rs): the Some branch resolves the object ids from the resolving
ability's already-populated `targets` via effect_object_targets(&filter,
&ability.targets) — the objects a preceding Effect::ChooseFromZone chose and
forwarded (CR 608.2c) — then manifest_card(…, cloaked_2_2()) per object. It does
NOT read a TrackedSet (never published for a Cloak continuation) and does NOT
touch the frozen effects/mod.rs. The None branch is the unchanged library-top
loop.

Parser: "cloak a card from your hand" lowers (at the intercept level, mirroring
the SearchLibrary sub_ability precedent — a bare Effect can't express a chain)
to a composite ChooseFromZone{zone:Hand,count:1} parent + Cloak{object_source:
Some(ParentTarget)} sub_ability, reusing the fully-wired ChooseFromZoneChoice
interactive stack (no new WaitingFor/AI/frontend). A `from_zone: Option<Zone>`
discriminant on ImperativeFamilyAst::Cloak distinguishes hand-source from
library-top. Pure nom (tag/alt/all_consuming).

Walker (phase-rs#4904 ability_scan.rs): the compile-forced Cloak arm gains a guarded
`if let Some(f) = object_source { acc = acc.or(scan_target_filter(f)); }`.

Anti-hollow-win test (tests/vannifar_cloak_from_hand.rs): drives the real
resolve_ability_chain → ChooseFromZoneChoice → apply(SelectCards[A]) → cloak
path; asserts the chosen hand card A is cloaked (face_down 2/2 ward{2}, leaves
hand) while the distinguishable library-top card B is UNTOUCHED. Revert-to-red
proven twice — point object_source at library-top → B cloaked / A stays (RED at
"A must be cloaked"); revert the intercept → Unimplemented. Plus back-compat
(library-top unchanged, object_source:None asserted explicitly) and negative
sibling. Expose the Culprit remains a separate later gate (reuses this field).

Full test-engine exit 0; CI clippy 0 warnings; coverage flip Vannifar
supported gap_count 1→0, REGRESSED(engine)=0 GAINED=1; semantic-audit clean.
CR 701.58a / 701.58e / 608.2c (grep-verified).

Assisted-by: ClaudeCode:claude-opus-4.8
matthewevans added a commit that referenced this pull request Jul 5, 2026
…#5155)

* feat(engine): dynamic keep count on the dig pipeline (Stargaze)

Parameterize PutCount::Up/Exactly payload u32 -> QuantityExpr and add an
additive Effect::Dig.keep_count_expr so "put X cards from among them"
(dynamic keep) both lowers and resolves. Unlocks Stargaze and the whole
"look at N, put <dynamic> into hand, rest into Y" class. The look count
(twice X) already resolved; the dynamic keep was the sole blocker.

Reusable building block: single-authority PutCount::to_dig_keep mapping;
keep resolved against game state before WaitingFor::DigChoice; additive
serde-default field keeps existing fixed-count Dig snapshots byte-identical.

CR 701.20e (look), CR 608.2c (follow instructions), CR 107.1b (negative -> 0).

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): opponent-constrained target-player slot (Quick Draw)

Add ControllerRef::TargetOpponent -- a pure routing tag whose runtime read
is identical to TargetPlayer but whose companion target-player slot offers
only opponents (self excluded; any one opponent in >2p), reusing the
existing TargetFilter::Typed{controller:Opponent} + find_legal_targets
legality path. Lowers "creatures target opponent controls lose <kw>..."
(Quick Draw) and unlocks the whole "target opponent controls" class.

Shared walker effect_bound_filter_matches feeds both target-player and
target-opponent detection; relative_controller_kind normalizes
TargetOpponent -> TargetPlayer so the fanout/rewrite subsystem reuses
unchanged; the silent spell-filter wildcard is closed to fail-closed.

CR 109.4, CR 102.2 / CR 102.3 (opponent), CR 611.2c (EOT set fixed at start),
CR 702.7a (first strike), CR 702.4a (double strike).

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): reflexive "this way" delayed-trigger building block (Prishe, Rhino)

Add DelayedTriggerLifetime::Reflexive (CR 603.12) — reflexive triggered
abilities are checked immediately after being created, firing on whether the
trigger event occurred earlier during the same resolution. Generalizes the
former coin-flip-only discard special case: reflexive_coin_flip_resolved_without_match
is removed and replaced by lifetime-keyed is_reflexive_lifetime, and
build_reflexive_coin_flip_trigger now emits Reflexive so coin flips route
through the same rule.

Parser gains a nom detector on the disjoint " this way, " delimiter
(try_parse_reflexive_this_way_trigger) plus a damage-first recognizer
(parse_reflexive_excess_damage_trigger, CR 120.10 excess-damage). Zone-change
"this way" reflexives stay deferred via the strip_if_you_do_conditional guard;
unknown conditions remain honestly Unimplemented.

Cards: Prishe's Wanderings (search-library reflexive -> +1/+1 counter),
Rhino's Rampage (excess-damage-first reflexive -> destroy up-to-1, scoped to
And[ParentTarget, opponent-controlled]).

Tests: 4 runtime pipeline tests (2 positive, 2 discard revert-failing on
Reflexive->ThisTurn) + 2 parser round-trips; migrated the breeches coin-flip
integration test to the Reflexive lifetime.

Assisted-by: ClaudeCode:claude-opus-4.8

* fix(engine): scope "lose control" trigger + emit control-loss at cleanup (CR 514.3a)

match_changes_controller fired on ANY ControllerChanged/EffectResolved{GainControl},
ignoring valid_card and direction — a latent over-fire (Portent trap) for the three
supported "When you lose control of ~" cards (Khârn the Betrayer, Duplicity,
Gustha's Scepter). Scope it to ControllerChanged, gated by valid_card, and resolve
direction by source identity:
  - self-ref ("~"): the source IS the changing object, whose controller has already
    flushed to new_controller by trigger-scan time (flush_layers runs at the top of
    collect_pending_triggers). Rely on CR 603.10d look-back — a loses-control ability
    is intrinsically the pre-change controller's, and old != new already guarantees
    exactly one loser. Fire for it.
  - delayed/SpecificObject (Stolen Uniform): the source is the graveyard spell whose
    controller stays constant (the temp holder), so old_controller == source.controller
    fires on the loss (old == caster) and not the initial gain (old == owner). CR 603.2.
Deliberate rules-correctness flip for the three cards: they no longer over-fire on
unrelated control changes or gains; the correct "that permanent leaves your control"
case still fires.

Targeted GainControl::resolve now emits ControllerChanged (mirroring GainControlAll
and GiveControl) so dropping the matcher's redundant EffectResolved arm cannot regress
a loss.

execute_cleanup emits ControllerChanged when an until-end-of-turn control effect ends
(CR 514.2), fires delayed triggers on that loss before the this-turn prune, and hands
back priority; priority.rs re-enters cleanup once the stack empties (CR 514.3a "another
cleanup step begins"). This is the runtime half that lets a future "when you lose
control of that <permanent> this turn" delayed trigger (Stolen Uniform) fire; the
card's parser front half is not yet supported and stays honestly Unimplemented.

Tests: 4 runtime tests driving the real cleanup/priority/dispatch pipeline (3 delayed
SpecificObject + 1 self-ref), each revert-probed RED against the exact gate it covers.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): look-at + play face-down exile building block (Outrageous Robbery)

Outrageous Robbery: "Target opponent exiles the top X cards of their library
face down. You may look at and play those cards for as long as they remain
exiled. If you cast a spell this way, you may spend mana as though it were mana
of any type to cast it."

Adds casting::player_may_look_at_facedown_exile — the single authority for "may
this player look at this face-down exiled card?", delegating to
play_from_exile_permission_source so look- and play-permission cannot diverge
(CR 406.3b: a face-down exiled spell may be cast only if the player is allowed
to look at it). It inherits the source's card_filter / single_use / per-turn
gating. visibility.rs face-down-exile redaction consumes it as a third
look-permission class alongside foretell and hideaway. The play grant carries
mana_spend_permission: Some(AnyTypeOrColor) (CR 609.4b), read by the existing
play-from-exile payment path; the reveal turns the card face up on cast
(CR 406.3a).

Parser: the subject-voice "<player> exiles the top N ... face down" arm now
(a) resolves the cost's X to Variable("X") (was Fixed(1)), (b) honors a trailing
"face down", and (c) scans "as though it were mana of any type" (was color-only)
so the rider folds onto the play grant. Corrects a pre-existing wrong CR
annotation on that arm (701.10a Doubling -> 701.13a Exile). swallow_check
recognizes the folded PlayFromExile{mana_spend_permission: Some(_)} as the
structural form of the "if you cast a spell this way" rider, suppressing a
Condition_If false positive (also covers Brainstealer Dragon).

Tests: 3 runtime tests (grant lands face-down + any-type on real cast pipeline;
look-permission is grant-scoped — grantee sees the cards, owner and other-source
face-down exiles stay hidden; permission persists across the turn) + 1 parser
shape test. Off-color cast-from-exile payment proven by the shared PlayFromExile
consumer-arm test.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): parse "the spell's mana value <= X" gate for impulse-cast (Bre of Clan Stoutarm)

Bre of Clan Stoutarm's end-step ability: "if you gained life this turn, exile
cards from the top of your library until you exile a nonland card. You may cast
that card without paying its mana cost if the spell's mana value is less than or
equal to the amount of life you gained this turn. Otherwise, put it into your
hand." The only unsupported piece was the trailing mana-value gate: it was
silently dropped, which cascaded the "Otherwise" clause into Unimplemented.

Adds the nom combinator parse_offered_card_mana_value_comparison — "[the|that]
spell's/card's mana value is {less/greater than [or equal to]} <quantity>" ->
StaticCondition::QuantityComparison { ObjectManaValue{Target} <cmp> <quantity> }
(CR 202.3 mana value, CR 115.1 target, CR 608.2c). It is hard-anchored on the
demonstrative prefix so it does not overlap the reflexive "its mana value is N"
(ObjectManaValue{Recipient}) or the "with mana value N" filter forms. Once the
gate re-homes onto the cast clause, the pre-existing else_ability machinery
routes "Otherwise" to hand automatically — no new Effect, no runtime change; the
ExileFromTopUntil / CastFromZone{without_paying_mana_cost} / LifeGainedThisTurn
building blocks were already wired. Bre's activated ability already parsed.

Tests: 4 runtime tests on the real trigger->resolution pipeline — free-cast when
MV <= life (revert-failing on the combinator), to-hand when MV > life, no-op when
no life gained (intervening-if), and a decline-while-eligible -> hand
CHARACTERIZATION test. The decline case documents a known interpretive edge: the
engine's else_ability convention routes both condition-false and optional-decline
to hand, whereas a strict reading of "Otherwise" (= MV>life only) would leave a
declined-but-eligible card exiled; no published Bre ruling either way as of
2026-07-02, tracked as class-wide engine debt (Bre/Wick/Chandra).

Assisted-by: ClaudeCode:claude-opus-4.8

* fix: thread keep_count_expr + TargetOpponent through non-engine consumers (mtgish-import, phase-ai tests)

Commits 1b67f0248 (Stargaze) and 6f3a35d11 (Quick Draw) added the `keep_count_expr` field to `Effect::Dig` and the `ControllerRef::TargetOpponent` variant to the engine but did not update the non-engine consumer crates, leaving CI's exact lint surface (`cargo clippy --workspace --exclude phase-tauri --all-targets`) red. `cargo check --workspace` and `cargo test -p engine` both miss this: nextest excludes mtgish-import, and a plain `check` skips test targets.

- mtgish-import action.rs: add `keep_count_expr: None` to all 11 `Effect::Dig` initializers (fixed keep-count; the dynamic-keep override is populated only by Stargaze-class digs this crate does not convert).
- mtgish-import player_effect.rs: add the `ControllerRef::TargetOpponent` arm to controller_to_scope, strict-failing with EnginePrerequisiteMissing (a single targeted opponent has no broadcast ProhibitionScope; mapping it would over-broaden the prohibition).
- phase-ai control.rs + spellslinger_prowess.rs: add `keep_count_expr: None` to the 4 `Effect::Dig` initializers in `#[cfg(test)]` fixtures (only compiled by clippy --all-targets).

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): Memory Vessel — play-from-exile grant + can't-play-from-zone prohibition

Memory Vessel ({T}, exile it: each player exiles the top 7, may play them until the activator's next turn, and can't play from their hand) now lowers fully — the previously-collapsed "players may play cards they exiled this way, and they can't play cards from their hand" clause resolves into a per-owner play-from-exile grant plus a play-from-zone prohibition.

Engine (building blocks, reused across the class):
- ProhibitPlayFromZone { zone } on ProhibitedActivity — a DENY axis covering both casting and land plays (CR 116.2a/305.1/601.2a), distinct from the CastOnlyFromZones allow-list which is cast-only. Enforced at the cast gate, the play-land gate, and the castable-surface filter; the multiplayer HUD filter handles the new variant.
- The untap-step prune keys "until your next turn" expiry on the granting ability's controller via the existing exiled_by_ability_controller field (CR 514.2/611.2a), so a per-owner grant expires at the ACTIVATOR's next turn, not each grantee's — mirroring the end-step prune already used by Rocco, Street Chef. No new field.

Parser (nom combinators, build-for-the-class):
- parse_per_owner_exiled_this_way generalizes the ObjectOwner grant arm to "[each player|players] may [play|cast] [the] card[s] they exiled this way" (covers Rocco + Memory Vessel).
- try_parse_cant_play_from_zone: "[scope] can't play [cards|lands...] from [zone]" -> ProhibitPlayFromZone (also flips Shaman's Trance's graveyard prohibition — a class win).
- try_parse_exile_play_grant_with_play_prohibition composes the two under a shared leading duration.

Tests: card-level parse assertion (0 Unimplemented) + 3 multiplayer runtime tests (per-owner scoping, activator-keyed cross-player expiry, can't-play-from-hand blocks both cast and land while exile plays stay legal).

Empirical corpus coverage diff (2555-card superset): net Unimplemented -2 (Memory Vessel + Shaman's Trance flip supported), zero regressions.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): dual-target slot registry + anaphoric slot binding (Stolen Uniform front-half)

Add a declared-target-slot registry on ParseContext so "Choose target X and
target Y" chains bind later anaphors ("that Equipment", "the chosen creature",
"the artifact card") to a precise ParentTargetSlot{index} instead of the
ambiguous grab-all ParentTarget. Generalizes the Goblin-Welder hardcoded
artifact-slot resolver into a type-driven registry lookup (hardcoded arms
deleted; Goblin Welder reproduced via the general path). GainControl and Attach
now bind slot-precisely (Slot1 / {attachment:Slot1, target:Slot0}).

Also wires the ParentTargetSlot arm into Attach's resolve_object_filter and
GainControl's gain_control_object_targets — their bespoke object-resolution
paths lacked it — reusing targeting::resolve_parent_slot_from_root (root-chain
nth), which incidentally fixes timmerian fiends (its "the artifact card" was
wrongly bound to a non-existent slot).

Front-half only: Stolen Uniform's last sentence ("When you lose control of that
Equipment this turn ... unattach it") stays Effect::unimplemented pending the
block-D delayed-trigger container; card is not yet supported.

The determiner anaphor path uses nom combinators; the pre-commit parser gate's
flags are stale-base (ae663ee8c) false positives on pre-existing mod.rs /
untouched oracle_trigger.rs lines — none in this commit's additions (verified).

CR 601.2c (target chosen once per instance of "target") + CR 608.2c (later
instructions reference earlier objects via whole-chain accumulation).

Assisted-by: ClaudeCode:claude-opus-4.8

* fix(engine): classify new engine surfaces in the #4904 fail-closed walker

Rebasing the S25 tranche onto main (#4904 growing-cascade detector + fail-closed
ability-scan walker) surfaces two exhaustive-match classification points for
engine surfaces this branch added before the walker existed:

- ControllerRef::TargetOpponent (Quick Draw) — Axes::NONE in the C0 axis
  classifier, mirroring TargetPlayer. The two are runtime-read-identical; the
  opponent-only legality is enforced at target selection, not a walker axis
  (CR 109.4).
- Effect::Dig.keep_count_expr (Stargaze) — scanned via scan_quantity_expr in the
  C0 axis classifier. A dynamic keep-count is a projected-resource read (axis 3),
  scaling with game state exactly like the dig `count`, so it must feed the
  growing-cascade detector identically rather than being ignored.

keep_count_expr also passes through effect_resolution_choice_freedom's Dig arm,
which classifies Dig as MayPrompt (fail-closed) — a keep-count adds no priority
WaitingFor, so the {..} pass-through is sound and needs no per-field action.

ProhibitedActivity::ProhibitPlayFromZone (Memory Vessel) and
DelayedTriggerLifetime::Reflexive (Prishe/Rhino) are not walker-traversed; their
own commits already handle their exhaustive match sites.

Only ability_scan.rs (game/) changed; the pre-commit parser gate's flags are
stale-base (ae663ee8c) false positives on pre-existing parser lines not in this
commit.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): pay turn-face-up cost as a special action (Overgrown Zealot, Tin Street Gossip)

Turning a face-down permanent face up is a special action (CR 116.2b) that must
pay the morph/megamorph/disguise cost (CR 702.37e / 702.168d) or the manifested
creature's mana cost (CR 701.40b). The engine previously performed the flip for
free, so mana abilities whose only live branch is "produce mana usable only to
turn a permanent face up" (CR 106.6 restricted-purpose mana) were left as
honest-red Unimplemented gaps.

- morph.rs: split the guards + cost extraction out of `turn_face_up` into
  `turn_face_up_prepare` (no signature change to turn_face_up; its ~8 free
  callers stay free — the payment lives in the handler, not the primitive).
- engine.rs: the GameAction::TurnFaceUp handler now reduces + pays the cost via
  `pay_special_action_mana_cost(..., SpecialAction::TurnFaceUp, ...)`, mirroring
  the UnlockDoor special-action sibling. Sole production paid entry.
- types/ability.rs: `has_payable_branch(TurnPermanentFaceUp)` flips dead→live so
  the sequence-absorption seam now absorbs the restriction into a real
  Effect::Mana (FaceDownSpell stays dead — CR 702.37c face-down casting is still
  unimplemented). Monotonic MORE→true: only the 3 TurnPermanentFaceUp cards are
  affected (Overgrown Zealot + Tin Street Gossip gain support; Creeping Peeper
  already supported via SpellType/UnlockDoor, unchanged).

Discrimination proven empirically: reverting the handler payment flips R1/R2/R3
red; R2 (empty pool → Err, permanent stays face_down) is the load-bearing charge
proof. No new enum/field (existing ManaSpendRestriction / SpecialAction::TurnFaceUp
/ PaymentContext::SpecialAction) → zero consumer-crate churn. s07-frozen files
untouched. Parser dispatch unchanged (oracle_tests.rs changes are test assertions
only; the gate's flags are stale-base ae663ee8c false positives, not this commit).

CR 106.6 + CR 116.2b + CR 702.37e + CR 702.168d + CR 701.40b + CR 702.37c.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): grant abilities beyond activated-only via GrantedAbilityScope (Symbiote Spider-Man, Choreographed Sparks, Nalfeshnee)

Parameterize Effect::GainActivatedAbilitiesOfTarget with a typed
GrantedAbilityScope { ActivatedOnly (default) | AllOther } field (serde-default,
zero consumer-crate churn) instead of adding a sibling variant — the granted
ability set is fixed at effect start (CR 611.2c), within a single rule section.

- AllOther snapshots BOTH the object's activated abilities AND its separate
  trigger_definitions store (CR 603.1 — triggered abilities are a distinct
  ability class the prior activated-only loop never read), granting each and
  excluding the granting ability itself; the grant is permanent (CR 611.2a).
- The resolver branches on the donor filter: Symbiote Spider-Man inverts the
  axes (donor = this card via SelfRef, recipient = the +1/+1 target via
  ParentTarget) vs the existing mirror.
- Choreographed Sparks / Nalfeshnee: apply_spell_copy_modifications now stamps
  AddKeyword + GrantTrigger onto both the base and live stores (they were
  silently dropped), so "the copy gains haste and a sacrifice trigger" persists
  across the copy→token boundary. The parser fold lands at lower_effect_chain_ir
  (the chain chokepoint shared by ability and trigger-execute chains), so
  Nalfeshnee — whose grant lives in a triggered ability — flips too.

Walker: the new scope field is a static ability-kind selector (no game-state
read) → Axes::NONE in the fail-closed ability-scan classifier.

Discrimination proven empirically: reverting the AllOther trigger snapshot flips
S1/S2/S3 red; disabling the copy-modification fold flips Choreographed + Nalfeshnee
red. Coverage +3, zero regressions across all 35397 faces. No new Effect variant;
s07-frozen files untouched.

CR 611.2a + CR 611.2c + CR 603.1 + CR 701.21a (delayed sacrifice) + copy/haste rules.

Assisted-by: ClaudeCode:claude-opus-4.8

* test(engine): flip Choreographed Sparks deferred-pin to supported

P2f (4b2566eb6) implemented Choreographed Sparks' copy-grant (haste +
delayed-sac fold via apply_spell_copy_modifications), invalidating the
deferred-honesty guard `choreographed_sparks_copy_grant_is_deferred`, which
asserted the card still lowered to Unimplemented. Flip it to a supported
regression guard (`_is_supported`, asserts NOT Unimplemented).

This integration test lives in tests/ and was missed by P2f's
`cargo test -p engine --lib` run; the full `cargo test -p engine` gate catches
it. Fixup for 4b2566eb6 — fold at ship-time autosquash.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): Stolen Uniform lose-control container — runtime-correct unattach delayed trigger (#4380 block-D)

Parser recognizer for Stolen Uniform's last sentence ("When you lose control of
that Equipment this turn, if it's attached to a creature you control, unattach
it") plus the two engine gaps it exposed, so the delayed trigger actually
unattaches the right Equipment at cleanup — not a hollow parse flip.

Parser (oracle_effect/mod.rs, oracle_target.rs): new nom-only
try_parse_lose_control_delayed_trigger emits
CreateDelayedTrigger{ ChangesController, ThisTurn, valid_card: ParentTargetSlot{1} }
with effect UnattachAll{ attachment: ParentTargetSlot{1}, target: Typed{Creature, You} }
(intervening-if folded into the host scope). Fires only on "when you lose control
of " + a resolvable dual-target-registry anaphor, so no lose-control sibling
regresses (only Stolen is dual-target). CR 603.7 / 603.4 / 603.2 / 701.3d.

Gap #1 — trigger stall (triggers.rs): UnattachAll is a non-targeted mass effect,
but extract_target_filter_from_effect surfaced its host filter as a required
target slot, so the delayed trigger paused on an unresolvable pick and never
resolved. Carve it out like Sacrifice / at-resolution Bounce; matches the None
its mass siblings (DestroyAll/BounceAll) return from Effect::target_filter.
CR 701.3d + CR 115.1.

Gap #2 — attachment anaphor (attach.rs): resolve_unattach_all passed the raw
ParentTargetSlot{1} to matches_target_filter, which returns false for positive
parent-refs by design (resolve at resolution time). Resolve the context-ref
attachment against the ability's target snapshot via effect_object_targets,
mirroring resolve_attach. Closes the divergence for the whole ParentTarget /
ParentTargetSlot "attach/unattach it" class. CR 608.2c + CR 701.3d.

Depends on the s07 delayed-trigger root-chain snapshot fix (a410d2d74, picked as
ccbfc4b4b) so ParentTargetSlot{1} snapshots [C, E].

Tests: stolen_uniform_lose_control_unattaches_only_that_equipment (E unattached,
hostile F stays — slot-specific) + _fold_leaves_opponent_hosted_equipment
(host-scope discriminator, now non-vacuous) un-ignored and green;
extract_target_skips_unattach_all building-block unit test (revert-fails if the
gap#1 carve-out drops). Full test-engine: 14707 lib + all integration green;
CI clippy 0 warnings.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): become-a-typed-token copula on reanimated objects (Vraska the Silencer, Brilliance Unleashed)

P2e — "It's a <typed thing> …" applied to a returned/reanimated non-copy
object, as an indefinite continuous effect bound to that object. Reuses the
copy path's SetCardTypes + subtype + granted-ability builder; zero new engine
variants, zero resolver changes, zero frozen-file edits.

Vraska, the Silencer: "return that card … tapped under your control. It's a
Treasure artifact with '{T}, Sacrifice this artifact: Add one mana of any
color,' and it loses all other card types." The copula routes to the shared
parse_its_a_type_loses_others builder (now pub(super)) via a new arm in
subject.rs, gated on ParentTarget | TriggeringSource (declines SelfRef so a
source-permanent misbind honest-defers). The dies-trigger return binds
TriggeringSource, which the existing register_transient_effect arm resolves to
the returned object — no publish-as-ParentTarget needed. CR 205.1a / 205.1b /
613.1d / 611.2a / 400.7 / 603.6 (NOT 707.9d — copy-effect-scoped).

Block 1a (sequence.rs): the bare " and " sequence splitter bisected the copula
before "…and it loses all other card types", hiding the CR 205.1a replacement
signal from the builder. Suppress the split when the remainder is exactly
"(it )?loses all other card types" — class-scoped to the replacement copula
(all 16 such cards unchanged; coverage REGRESSED(engine)=0).

Brilliance Unleashed (mode 2 Otherwise): "Otherwise, return it to the
battlefield and it's a 3/3 Robot artifact creature with flying." The Otherwise
else is a fresh recursive effect chain whose clause list starts empty, so the
typed referent from "Choose target artifact card" was lost and the animation
copula declined to Unimplemented. Seed ctx.parent_target_available across the
else recursion (behind a skip_first_conditional param; both pre-existing callers
pass false so the second consumer is byte-unchanged) and scope-rebind the
reanimate-else animation duration to UntilHostLeavesPlay. Additive by
construction — can only turn a declining anaphor into ParentTarget, never remove
a binding. Bre of Clan Stoutarm (C12) reuses the same else-seed.

C3: both copulas install Duration::UntilHostLeavesPlay (CR 400.7 — a returned
object is a new object; mirrors install_aura_continuous_effect).

Tests (std_longtail_e.rs, +4, deferred note flipped): parser round-trip +
runtime for each card. The Vraska runtime test asserts the TCE binds
SpecificObject{returned_id} (not Vraska via a use_self misbind, not inert),
returned obj is Artifact+Treasure, and the granted ability sacrifices SelfRef —
all revert-to-red proven. gen-card-data: both flip supported, gap_count=0.
Full test-engine 16924/0; CI clippy 0 warnings; coverage REGRESSED(engine)=0;
semantic-audit clean.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): cloak from a chosen non-library source (Vannifar, Evolved Enigma)

Vannifar's "Cloak a card from your hand" — the controller cloaks a card they
CHOOSE from hand, not the top of the library. Adds a source axis to Cloak and
threads the chosen object through the resolver so it cloaks the right card, not
a hollow library-top flip.

Parameterize (not proliferate): add `object_source: Option<TargetFilter>` FIELD
to Effect::Cloak (`#[serde(default, skip_serializing_if="Option::is_none")]`).
None = CR 701.58e library-top source (Cryptic Coat, Ransom Note — byte-identical
serialization, back-compat proven). Some(filter) = explicit objects chosen
upstream. Zero new variants.

Resolver (cloak.rs): the Some branch resolves the object ids from the resolving
ability's already-populated `targets` via effect_object_targets(&filter,
&ability.targets) — the objects a preceding Effect::ChooseFromZone chose and
forwarded (CR 608.2c) — then manifest_card(…, cloaked_2_2()) per object. It does
NOT read a TrackedSet (never published for a Cloak continuation) and does NOT
touch the frozen effects/mod.rs. The None branch is the unchanged library-top
loop.

Parser: "cloak a card from your hand" lowers (at the intercept level, mirroring
the SearchLibrary sub_ability precedent — a bare Effect can't express a chain)
to a composite ChooseFromZone{zone:Hand,count:1} parent + Cloak{object_source:
Some(ParentTarget)} sub_ability, reusing the fully-wired ChooseFromZoneChoice
interactive stack (no new WaitingFor/AI/frontend). A `from_zone: Option<Zone>`
discriminant on ImperativeFamilyAst::Cloak distinguishes hand-source from
library-top. Pure nom (tag/alt/all_consuming).

Walker (#4904 ability_scan.rs): the compile-forced Cloak arm gains a guarded
`if let Some(f) = object_source { acc = acc.or(scan_target_filter(f)); }`.

Anti-hollow-win test (tests/vannifar_cloak_from_hand.rs): drives the real
resolve_ability_chain → ChooseFromZoneChoice → apply(SelectCards[A]) → cloak
path; asserts the chosen hand card A is cloaked (face_down 2/2 ward{2}, leaves
hand) while the distinguishable library-top card B is UNTOUCHED. Revert-to-red
proven twice — point object_source at library-top → B cloaked / A stays (RED at
"A must be cloaked"); revert the intercept → Unimplemented. Plus back-compat
(library-top unchanged, object_source:None asserted explicitly) and negative
sibling. Expose the Culprit remains a separate later gate (reuses this field).

Full test-engine exit 0; CI clippy 0 warnings; coverage flip Vannifar
supported gap_count 1→0, REGRESSED(engine)=0 GAINED=1; semantic-audit clean.
CR 701.58a / 701.58e / 608.2c (grep-verified).

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): cloak an exiled face-down pile — Expose the Culprit mode 2

Expose the Culprit's mode 2 ("Exile any number of face-up creatures you
control with disguise in a face-down pile, shuffle that pile, then cloak
them") lowers to ChooseObjectsIntoTrackedSet{Creature, You,
HasKeywordKind{Disguise}} -> Shuffle{TrackedSet} -> Cloak{object_source:
Some(TrackedSet)}.

- KeywordKind::Disguise (CR 702.168): a discriminant-level keyword kind
  (like Morph/Megamorph) so HasKeywordKind{Disguise} names the class
  regardless of the Disguise(ManaCost) payload. The "with disguise"
  filter selects only face-up disguise creatures — a face-down permanent
  has no keywords (CR 708.2a).
- Shuffle gains a TrackedSet/pile branch (CR 701.24a): randomizes the
  chain's tracked object set via the RNG and emits no ShuffledLibrary
  action, so a pile shuffle does not fire library-shuffle triggers.
- Cloak gains a TrackedSet source: manifest_card on a battlefield
  permanent is a no-op (the Battlefield->Battlefield zone guard), and the
  card literally exiles then cloaks, so each chosen creature is EXILED (a
  real Battlefield->Exile move — CR 122.2 counters cease, CR 603.6c
  leaves-the-battlefield triggers fire, CR 704.5m/704.5n Auras and
  Equipment fall off) then manifested back from exile as a fresh
  face-down 2/2 with ward {2} (CR 400.7 new object, CR 701.58a/e). The
  cloak reads the shuffled tracked set directly so pile order is
  observable.

Runtime tests drive the full cast chain and prove: chosen creatures
become face-down 2/2s in the shuffled (non-selection) order; no
ShuffledLibrary event; a +1/+1 counter is cleared and an attached Aura
falls to the graveyard after the object reset; and an empty selection is
inert.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): interactive "any number" multi-zone name-matched search-and-exile

Route "search <its owner's/its controller's> graveyard, hand, and library for
any number of cards with <same-name-ref> and exile them" to the interactive
Effect::SearchLibrary path (CR 701.23b — a search for a stated quality lets the
player fail to find), covering Deadly Cover-Up, The End, Crumble to Dust,
Surgical Extraction, Test of Talents, and Deicide. Zero new engine enum variants.

Generalizes the existing multi-zone same-name recognizer's quantifier axis
(all cards | any number | up to N) and branches the lowering: "all cards" keeps
the mandatory ChangeZoneAll; the interactive quantifiers lower to
SearchLibrary{SameNameAsParentTarget}. An object-relative possessive guard
(its owner's / its controller's only) keeps the chosen-name class (Unmoored Ego,
Memoricide, ... — "choose a card name ... with that name") on the HasChosenName
path (CR 201.2).

- W4: resolve_library_owner resolves a Typed(ParentTargetOwner/Controller)
  searched-player via controller_ref_player; the caster remains the searcher
  (CR 701.23a asymmetric); bare ParentTargetController (Assassin's Trophy) untouched.
- W5: a found-set hand-exile counter at SearchChoice completion feeds the "draws
  a card for each card exiled from their hand this way" rider (CR 121.1).
- W6: apply_anchor_subject gains a Draw arm so the rider draws to the searched
  player, not the caster.
- target_filter(): a bare Typed(ParentTargetOwner/Controller) searched-player is a
  resolution-time context-ref (like RevealUntil), not a cast-time target slot.

Tests: 6 discriminating parser tests + 3 runtime cast tests (The End controller
axis; Deadly Cover-Up owner axis, Case A/B with evidence gating; Test of Talents
countered-spell seed), each with revert-to-red evidence.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): bind granted-ability self-references to the granting object (CR 201.5a)

When an ability grants another ability that refers to the granting object by
name (Deconstruction Hammer "Sacrifice Deconstruction Hammer", The Dominion
Bracelet "{15}, Exile The Dominion Bracelet", Trusty Boomerang "Return Trusty
Boomerang"), CR 201.5a says the name refers only to the granting object, never
the host it was granted to. Previously these self-references collapsed to the
host (~/SelfRef), so an equipment's granted sacrifice/exile/return acted on the
equipped creature instead of the equipment.

New parse-time TargetFilter::GrantingObject, emitted only in self-reference
verb-object positions (sacrifice/exile/return/put-counter-on <self> inside a
quoted granted body), concretized to SpecificObject{granting object} at
grant-clone time (layers.rs GrantAbility/GrantTrigger, via effect.source_id).
Three channels stay separate: granter-referential -> GrantingObject; host-
referential "this permanent" -> SelfRef (host); host power read -> QuantityRef
(host). The name masker is allowlist-gated to verb-object positions so
out-of-scope in-quote self-name references (QuantityRef "counters on ~",
exclusion "other than ~", damage-source "by ~") stay byte-identical to the
pre-change baseline (coverage-regression: 0 regressed / 0 gained). A single
post-parse sweep degrades any residual placeholder to ~ in description strings.

Fixes the cost + effect-target channels: The Dominion Bracelet, Deconstruction
Hammer, Trusty Boomerang, Razor Boomerang, Fishing Pole, Hankyu, Spare Dagger,
Sakashima. The QuantityRef/condition/damage-source/exclusion granter-name
channel remains host-bound (pre-existing; flagged in-code as a CR 201.5a
follow-up). The Dominion Bracelet's {X}-less cost reduction folds into
cost_reduction (host-referential power, CR 601.2f). Grant-time concretization
snapshots the granter id and is scoped in-code to "no intra-resolution zone
move of the granter" (CR 201.5a second sentence + CR 400.7).

Zero new engine enum variants beyond TargetFilter::GrantingObject.

Tests: 10 discriminating tests (Hammer full activate/resolve zone check;
Bracelet exile + host-power reduction; Trusty bounce; Sliver "this permanent"
host-ref preserved; Food Fight "named" filter preserved; Archery Training /
Animal Friend / Torrent of Lava out-of-scope channels unmasked; description
no-leak; Fishing Pole + Hankyu counter-target), each with revert-to-red.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): phase-scoped player control for Secret of Bloodbending (CR 723.2)

Secret of Bloodbending: "You control target opponent during their next combat
phase. If this spell's additional cost was paid [waterbend {10}], you control
that player during their next turn instead." Previously the base "next combat
phase" leaf was Unimplemented — CR 723.2 limited-duration control had no
representation and the runtime could only release control at a turn boundary.

Parameterize Effect::ControlNextTurn with window: ControlWindow { NextTurn,
NextCombatPhase } (serde-default NextTurn — all existing fixtures/card-data load
unchanged). CR 723.1 full-turn control (Mindslaver, Worst Fears, Sorin,
Construct a Cosmic Cube) is untouched; only the phase-scoped window is new.

Runtime EXTENDS the single control machinery — no parallel schedule or
controller field. A new phase-boundary activate/release hook in
finish_enter_phase (turns.rs) binds control at the affected player's next
BeginCombat and releases at the following PostCombatMain/Cleanup, with a
release-before-activate ordering that makes "next combat phase" the first only
(CR 506.7d by analogy). One release authority, turn_control::release_control_at,
serves all three release sites: turn boundary (start_next_turn), combat-phase
boundary (finish_enter_phase), and leave-game (do_eliminate) — the last closing
a pre-existing CR 800.4a/b gap that also affected Mindslaver's full-turn control.

Parser: a window alt() axis on the control suffix combinator; a subject.rs
deferral guard so the full pipeline routes "control ... during their next combat
phase" to the imperative ControlNextTurn parser (it was mis-parsing "combat
phase" to Unimplemented via the subject-predicate path); the waterbend-paid
branch swaps to the NextTurn window via AdditionalCostPaidInstead; self-exile.

Edge cases designed (CR 723.1b + Scryfall ruling 2025-10-02): a skipped combat
phase carries; multiple combat phases bind the first only; the controlling or
controlled player leaving ends control (CR 800.4a/b); 3+ players route only the
controlled seat. Coverage-regression on fresh card-data: 0 regressed, +1 (Secret).

Tests: 8 groups incl. runtime cast-pilot (unpaid -> NextCombatPhase, paid ->
NextTurn + exile), control-active-exactly-within-combat, first-only latch, carry,
controller-leaves, 3+ player seat scoping — each with revert-to-red.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(parser): repeat a whole process a fixed/variable count of times (CR 608.2c)

"Then repeat this process X more times." (Another Round) previously left an
Unimplemented{repeat} node. Recognize the unconditional count form
"<q> more time[s]" in try_parse_repeat_process_directive and stamp the process
root's repeat_for = Offset{ <q>, +1 } (= "once + q more"), reusing the existing
ungated whole-chain repeat_for driver (repeated_full_chain, effects/mod.rs) —
zero engine, zero new variant. A prior /review-engine-plan rejected a proposed
new RepeatContinuation variant: RepeatContinuation is the non-count companion to
repeat_for, and the count belongs in repeat_for.

The recognizer uses the existing parse_quantity_expr_number combinator, so it
covers the whole class: "X more times" -> Offset{Variable(X),+1}, "six more
times" -> Offset{Fixed(6),+1}. Build-for-the-class, not one card — coverage:
Another Round and Professor Onyx flip to supported; Development improves. eof-
guarded so the conditional/stop/"once"/bare forms fall through unchanged
(coverage-regression on fresh card-data: 0 regressed, +2 gained).

CR 608.2c: the controller repeats the same instructions in order. CR 107.3a /
601.2b: X is announced at cast and fixed once. CR 400.7: each returned creature
is a new object (blink) — verified via summoning-sick re-entry per cycle.

Known follow-up (pre-existing, documented, not introduced here): "exile any
number of creatures you control" lowers to a cast-time target set, so each repeat
iteration re-blinks the same chosen creatures rather than re-choosing a fresh
"any number" per process (strict CR 608.2c). This is the shared cast-time-vs-
resolution-time selection quirk, out of scope for this card; the repeat count and
X+1 cycles are correct.

Tests: runtime cast (X=N,M creatures -> N+1 exile->return cycles; X=0 -> exactly
1) with live revert-to-red, plus a parser-shape guard. Parser-only.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(parser): reanimate self and a targeted graveyard card to the battlefield (CR 400.7)

"Return this card and target land card from your graveyard to the battlefield
tapped." (Sandman, Shifting Scoundrel) previously routed through the generic
shared-destination splitter, which wrapped SelfRef into a non-resolvable `And`
primary and left the verbless second conjunct Unimplemented — the whole ability
was inert (not even offered from the graveyard, because the `And` primary is not
a bare self-move so no activation zone was stamped).

Add a leaf recognizer `try_parse_reanimate_self_and_target` in
`lower_imperative_clause`, run before `try_split_targeted_compound`. It mirrors
the shipped Coastal Wizard / Lady Sun two-chained self-and-target idiom, adapted
to a battlefield destination: a BARE `ChangeZone { SelfRef }` primary (so
`activation_zone_from_self_effect` stamps the graveyard, Bloodsoaked Champion
precedent) plus a targeted `ChangeZone` sub_ability that reuses the full
return-to-battlefield lowering (origin inference, enter-tapped riders) from the
shared path rather than re-deriving them. The gate is self-validating — it fires
only when the second conjunct genuinely lowers to a non-battlefield-origin →
battlefield ChangeZone — so non-reanimation "return A and B" cards (e.g. Coastal
Wizard's return-to-hand) fall through unchanged.

Build-for-the-class: `strip_optional_target_prefix` recovers the "up to one
other target creature card" cardinality that the return-to-battlefield lowering
drops, so Slimefoot and Squee flips to supported too (multi_target = up_to(1),
optional, untapped). Coverage-regression on fresh card-data: 0 regressed, +2
gained (Sandman + Slimefoot exactly); no sibling "return A and B" card moved.

CR 400.7: each returned object is a new object; SelfRef names only the source
incarnation. CR 608.2c: the two chained moves resolve in written order.
CR 601.2c + CR 115.1: the graveyard card is a chosen target; SelfRef is not.
CR 113.6m: the bare self-move is what makes the ability function from (and be
offered in) the graveyard. CR 614.1: "to the battlefield tapped" enters both
objects tapped.

Tests: runtime activation from the graveyard (both objects return, both tapped;
the Land filter excludes a nonland and the unchosen land stays put — not a
sweep) with live revert-to-red (revert → null activation_zone → activation
illegal; sub stays Unimplemented → nothing moves), plus two parser-shape guards
and a coverage-honesty flip. Parser-only; no engine, no new variant.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(parser): disjunctive "first-of-type spell this turn" intervening-if (CR 603.4)

Alania, Divergent Storm's trigger — "Whenever you cast a spell, if it's the first
instant spell, the first sorcery spell, or the first Otter spell other than Alania
you've cast this turn, you may have target opponent draw a card." — left the whole
intervening-if plus the draw as an Unimplemented node. The already-built CopySpell
"if you do" sub was correct and is preserved untouched.

Recognize the three-way "first-of-type this turn" intervening-if by COMPOSITION,
not a bundled ordinal variant. Add one anchor-only leaf
`TriggerCondition::TriggeringSpellMatchesFilter { filter }` — the cast-spell "what
it IS" sibling of the existing match-event-subject cluster
(TriggeringSpellTargetsFilter / SourceMatchesFilter / ZoneChangeObjectMatchesFilter
/ EventDamageSourceMatchesFilter) — and compose each disjunct as
And(TriggeringSpellMatchesFilter(T), QuantityComparison(SpellsCastThisTurn{You,T}
== 1)), collected into TriggerCondition::Or. A prior /review-engine-plan rejected a
bundled TriggeringSpellIsNthCast{n,filter} as a layer-conflation (match axis + count
axis in one leaf) that also duplicates the existing SpellsCastThisTurn count; the
composition separates the layers and writes zero new count code, and is
behavior-identical at the CR 603.4 re-check.

The parser leaf recognizer (nom combinators, >=2-disjunct guarded so single-disjunct
cards stay on the untouched NthSpellThisTurn constraint path) emits that shape; the
"other than ~" self-exclusion lowers to Not(Named{card-name}). Build-for-the-class:
the anchor variant also unlocks the plain "if it's an instant spell" intervening-if.

Also fix a latent bug this exposed: spell_record_matches_filter dropped
TargetFilter::Named to the catch-all false, so Not(Named{X}) over spell history was
always true — silently no-opping any name self-exclusion (Alania's "other than ~").
Add the Named arm (record.name == name). Measured: zero existing cards fed a
top-level Named filter into a spell-record position, so this is a pure fix
(coverage-regression on fresh card-data: 0 regressed, +1 gained = Alania only; the
>=2-disjunct guard protected Vengevine and the 108 NthSpellThisTurn constraint cards).

CR 603.4: the intervening-if is checked at trigger time AND resolution — the count is
read live, so a second matching spell cast in response correctly fizzles it. CR
601.2a: the anchor keys on the SpellCast event's spell object. CR 201.2: card name
for the self-exclusion.

Tests: 6 runtime cast-pipeline tests (first-instant fires + copies; second-instant
same turn does not; first-sorcery fires; Otter self-exclusion by name; the CR 603.4
in-response fizzle; optional-draw decline skips the copy) each with live
revert-to-red, plus a parser-shape guard and a Vengevine constraint-path regression
anchor.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): Behold a [quality] as an interactive triggered-ability effect (CR 701.4a)

Sarkhan, Dragon Ascendant's ETB "you may behold a Dragon. If you do, create a
Treasure token." previously left an Unimplemented{behold} node — behold existed
only as a casting cost (AbilityCost::Behold), never as an effect. The already-
parsed Treasure sub (gated on OptionalEffectPerformed) and the second trigger
(Dragon-enters → +1/+1 + becomes-Dragon-with-flying) are preserved untouched.

Add Effect::Behold { filter }, an interactive resolution-time keyword action per
CR 701.4a ("Reveal a [quality] card from your hand or choose a [quality]
permanent you control on the battlefield"). The resolver (game/effects/behold.rs)
reuses the single candidate authority eligible_behold_choices (battlefield-you-
control ∪ matching hand) and a shared reveal_if_from_hand helper:
- 0 candidates: whiff — set cost_payment_failed_flag, stash no continuation, so
  the "if you do" rider reads performed && !flag = false (no Treasure).
- 1 candidate: forced, no agency — auto-select; a hand card emits CardsRevealed
  (card stays in hand), a permanent reveals nothing.
- >=2 candidates: a genuine, rules-visible choice (CR 608.2d) — park a new
  WaitingFor::BeholdChoice for the controller; the submit handler validates the
  chosen object is in the candidate set, reveals-if-hand, sets the rider
  performed, and drains.

A prior /review-engine-plan rejected an auto-resolve design: CR 701.4a grants the
player the choice, and a hand reveal is a public event, so auto-picking among two
or more distinct hand Dragons is an engine-made choice with rules-visible
consequences — pillar #1 (rules-correct over convenient).

Hidden-information correctness (CR 400.2): the BeholdChoice candidate list spans a
hidden zone (hand), so visibility.rs redacts the pre-choice candidates to an
opaque sentinel for opponents — otherwise the controller's matching hand cards
would leak before they choose. The post-choice reveal exposes only the chosen
card.

Behold has no stack target (chosen at resolution, not declared): target_filter()
is None, in the mass-effect group beside Clash/Populate.

The frontend BeholdChoiceModal is display-only — it renders the engine-provided
candidate list and dispatches a single-object SelectCards; it derives no
eligibility. i18n reuses the existing cardChoice.behold.* keys (present in all
seven locales from the cost-side UI) — zero new keys.

#5051 (CR 400.7): this fixed-quality behold writes no ChosenAttribute, so the
stale-chosen-type bug cannot manifest here; a code + test pin flags the future
type_choice axis for the fix sweep.

Tests: 7 runtime cast-pipeline tests (board-Dragon behold; hand-Dragon reveal
stays in hand; >=2-hand-Dragon interactive prompt with only-chosen-revealed;
opponent-view candidate redaction; accept-with-no-Dragon whiff; decline; parser
shape) each with revert-to-red. Coverage-regression on fresh card-data: 0
regressed, +1 gained (Sarkhan).

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): delayed sacrifice "at the end step on your next turn" (CR 513.2)

Kav Landseeker's "When this creature enters, create a Lander token. At the
beginning of the end step on your next turn, sacrifice that token." previously
left the delayed clause an Unimplemented{at} node — the parser had no temporal
arm for "the end step on your next turn", and the nearest existing condition
(AtNextPhaseForPlayer, "your next end step") fires the CURRENT turn's end step
(CR 513.2: a next-end-step delayed trigger created outside the end step is not
backed up), which would sacrifice the Lander before it could be used.

Add a turn-floor to AtNextPhaseForPlayer via a new typed enum
TurnGate { None, AfterCreationTurn, After(u32) } (not a bool, not a magic
sentinel): the parser emits the symbolic AfterCreationTurn, and
effects::delayed_trigger::resolve stamps it to After(creation_turn) at creation
(CR 603.7a) — the single-path binding site that already rewrites the PlayerId(0)
controller placeholder. The matcher then skips every matching phase up to and
including the floor turn and fires on the first strictly-later controller turn
(CR 500.7 extra turns included). An unstamped AfterCreationTurn reaching the
matcher debug_asserts and falls through to fire this turn (a loud, test-caught
wrong-timing signal) rather than silently never firing.

Existing AtNextPhaseForPlayer users (Greasefang, rebound, epic) default
gate: None and are byte-identical — None is skip_serializing_if, so only Kav
serializes a gate ("AfterCreationTurn", a pure semantic, no number). "That
token" reuses the existing TargetFilter::LastCreated snapshot-at-creation
(specific-token-safe per CR 603.7c / 400.7); the Lander predefined token and the
parsed Lander creation are untouched.

Tests: paired-timing runtime (Lander survives the current end step, is
sacrificed at the controller's next end step) with a resolved-stack reach-guard,
specific-token (a bystander token survives), a resolve-time stamp unit test, a
parser-shape guard, and a Greasefang non-perturbation guard — each with
revert-to-red. Coverage-regression on fresh card-data: 0 regressed, +1 gained
(Kav). Singleton delayed-trigger test scope is deliberate (see #5072).

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): Crowd-Control Warden dual enters/turned-face-up counter replacement + suppress own as-enters on face-down entry (CR 708.2a)

Crowd-Control Warden ("As this creature enters or is turned face up, put X +1/+1
counters on it, where X is the number of other creatures you control. Disguise …")
previously left the replacement line an Unimplemented node — the replacement
dispatcher matched the "As ~ enters" pattern but no inner parser handled the dual
"enters OR is turned face up" condition + the "put … on it" clause surface.

PARSER: recognize the self-and-face-up counters replacement (a new leaf
`lower_as_enters_or_face_up_counters`, wired at the Priority-8 replacement slot).
It emits two ReplacementDefinitions — one `Moved` (enter-the-battlefield) and one
`TurnFaceUp` — sharing one `PutCounter{P1P1, ObjectCount(other creatures you
control), SelfRef}` execute (CR 613.2a: both conditions generate the same effect).
The dynamic-count "on it" anaphor lowers to ParentTarget; the recognizer normalizes
it to SelfRef (matching the directly-built enters-with-counters shape) so the
runtime folds it as an ETB event modifier. A guard requires every execute effect to
be PutCounter{SelfRef}, so sibling "As ~ enters, choose…/becomes…" lines fall
through untouched. The dynamic count + turned-face-up runtime were already fully
supported (extract_etb_counters, turn_face_up_applier) — parser-only for the card.
Coverage-regression on fresh card-data: 0 regressed, +1 gained (Crowd-Control
Warden), blast radius exactly 1 card.

RUNTIME (the parser fix exposed a class bug): playing the Warden face-down via
Disguise wrongly gained the +1/+1 counters. CR 708.2a: a permanent that enters
face down is a 2/2 with no text, so its OWN "As ~ enters" replacement must not apply
on a face-down entry. Add a guard in object_replacement_candidate_applies:
`if is_entering && ZoneChange{face_down_profile: Some} return false` — is_entering
is true only when the candidate's source IS the entering object (its own text), so
EXTERNAL replacements (another permanent's enters-tapped) still apply. Masked until
now: existing morph/disguise cards with an own "enters with N counters" use an X
count (=0 face-down); the Warden's ObjectCount is the first nonzero one to expose
it. CR 708.3: the permanent is turned face down before it enters.

A pre-existing change_zone test (paused_face_down_change_zone_resumes_face_down_with_profile)
forced its replacement pause via the entrant's OWN SelfRef shock replacement on a
face-down entry — inadvertently asserting the masked bug. Rework it (test-only) to
force the pause via EXTERNAL replacements (mirroring the morph sibling), preserving
the load-bearing seam assertion (the PendingChangeZoneIteration carrier preserves
the face-down profile through pause/resume).

Tests: parser-shape (dual replacement, zero Unimplemented) + as-enters-choose
reach-guard; runtime face-down NEG discriminator (0 counters, reds to 2 on guard
revert) + hard-cast POS + face-down-then-turn-up; a no-over-suppression external
test + Hooded Hydra masked-class regression guards — each labeled by its
discriminating role. Full test-engine: 17263 passed.

Assisted-by: ClaudeCode:claude-opus-4.8

* chore(engine): rebase adaptation — classify S25 tranche variants into the #5072 walker

The #5072 PR-6.75 read/write conflict profiler (ability_rw.rs) is exhaustive over
Effect / TriggerCondition / TargetFilter / ControllerRef with no wildcards. This
tranche, written before #5072 merged, added new variants/fields the walker had no
arms for; rebasing onto main surfaced 13 compile errors (+ a latent Effect::Behold
masked by same-match E0027s). Classify each with its read/write profile and the
three ordering axes (reads_member_bound / reads_event_live / writes_event_object),
CR-grounded and mirroring the closest existing sibling:

- TargetFilter::GrantingObject (CR 201.5a): mirrors SpecificObject on the base
  axes; reads_member_bound = true (fail-closed R3 divergence). Parse-template-only —
  grant-clone concretizes it to SpecificObject{granter} (ability_utils.rs), so the
  arm is inert at runtime and classified conservatively.
- ControllerRef::TargetOpponent (CR 109.4): mirrors TargetPlayer (runtime-read-
  identical) — all axes empty (declared-target read, member-invariant).
- TriggerCondition::TriggeringSpellMatchesFilter (CR 601.2a/603.4): mirrors
  TriggeringSpellTargetsFilter — reads_event_live.
- Effect::Behold (CR 701.4a/608.2d): mirrors Reveal — a pure information event, no
  write; its resolution-time choice is classified in ability_scan (MayPrompt).
- Effect::Cloak.object_source (CR 701.58a), Effect::Dig.keep_count_expr
  (CR 701.20e/608.2c), Effect::GainActivatedAbilitiesOfTarget.scope (CR 602.1): new
  fields bound + profiled like their sibling fields.
TurnGate.gate (a match-time firing gate, not traversed by ability_rw) and
Effect::ControlNextTurn's ControlWindow (absorbed by the maximal-conservative arm)
need no binding.

Also adapts one lib test to a main API delta: #5072-era main replaced
Effect::DealDamage's former `excess_only: bool` with a typed `excess` field, so a
DealDamage constructor in copy_spell.rs's tests gains `excess: None` to match.

One tail adaptation commit rather than folding into the owning card commits: unlike
#5072's own case, the walker lives in the rebase BASE here, so folding WOULD restore
per-commit compiles — but a six-way fold across the tranche is error-prone; the arms
are grouped here and can be autosquashed at ship-prep if the final PR wants per-commit
bisectability. cargo check --workspace is clean (engine + phase-ai + mtgish-import +
server-core + wasm/server + test targets), zero downstream dual-walk sites, and the
full engine test suite passes (17549 tests, 0 failures) — no HIGH-2 / C2-ungating trip
in the tranche's tests (Kav's delayed-trigger tests were already singleton-scoped).

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): Rhys, the Evermore — interactive "remove any number of counters"

Implements Rhys's "{W},{T}: Remove any number of counters from target creature
you control" as a resolution-time interactive choice (CR 107.1c: "any number"
includes zero; CR 608.2d: the controller chooses at resolution).

Parser: a new "any number of" arm lowers the count to QuantityExpr::UpTo
(Fixed{-1}), reusing the existing UpTo slot — no new variant, so the #5072
read/write dual-walk needs no arms. A CR 608.2d "from among" guard keeps
out-of-scope multi-source removals (Galloping Lizrog, Eventide's Shadow) as
Unimplemented rather than collapsing them to single-source semantics.

Effect path: resolve_remove peels the UpTo flag and raises
WaitingFor::RemoveCountersChoice carrying the target's live per-type counter
counts; the controller's GameAction::ChooseCountersToRemove is validated by a
validate_counter_selection helper shared with the cost path and applied through
the CR 614.1 remove_counter_with_replacement pipeline via a pending_counter_removals
queue. The queue re-parks on replacement choices and, on drain, stamps
last_effect_count before the continuation drains, so "create that many" reflexive
clauses (Tetravus) read the true removed total (CR 608.2h).

Multiplayer: accepts_freeform_counter_removal + a session skip-legality arm let a
human submit an intermediate per-type selection the coarse AI candidate set does
not enumerate; the payload guard bounds the selection list.

Coverage: GAINED = exactly {Rhys}. Tetravus's remove-counters trigger and token
creation now resolve (proven: 3 counters -> 3 Tetravite tokens), but the card
stays unsupported on an unrelated keyword-grant clause nested in that trigger.
Cost-path storage lands / batteries and the multi-source cards are unchanged.

The plan reviewer's re-review request was folded into /review-impl with a mandate
to re-verify all six must-change resolutions at code level plus the four
non-compile-enforced registration points; verdict APPROVE.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): Esper Terra — "up to N" lore, conjunctive mana, token-anaphor counter binding

Fixes the two parser-leaf gaps on Esper Terra (Terra, Magical Adept // Esper
Terra) plus the shared binding that makes its marquee chapter effect land on the
right object. No new engine variant (reuses QuantityExpr::UpTo, PutCounter,
ManaProduction::Fixed, TargetFilter::LastCreated) — the #5072 read/write dual-walk
needs no arms.

Changes (all parser):
- Gap A (counter.rs): "put up to three lore counters on it" — strip the "up to "
  count marker and wrap the count in QuantityExpr::up_to (CR 608.2d + CR 122.1),
  mirroring the Draw/Discard/PutSticker convention. Count-side only; the target-side
  "on up to N target(s)" path is untouched.
- Gap B (mana.rs): "Add {W}{W}, {U}{U}, {B}{B}, {R}{R}, and {G}{G}" — a conjunctive
  comma+"and" fixed-mana-group accumulator (parse_fixed_mana_group_list) →
  ManaProduction::Fixed (CR 106.4). Rejects "or" lists, requires >=2 groups, no dedup.
- §B2 token-anaphor bind (context.rs + mod.rs + counter.rs): a bare "put ... counters
  on it" following a token creator binds to the created token, not the ability source
  (CR 608.2c). A ParseContext.token_created_in_chain signal (set from the most-recent
  prior referent, cleared by an intervening typed target) drives the it-pronoun counter
  branch to TargetFilter::LastCreated. A companion LastCreated guard on
  replace_target_with_parent's counter arm (mod.rs) preserves that bind through the
  lowering pass — the guard its sibling Attach/UnattachAll arms already carried.

The "if it's a Saga" gate needs NO code: it parses to TargetMatchesFilter{Saga}
reading ability.targets.first() (the copied enchantment, CR 707.2 type-equal to the
token), so a non-Saga copy places zero lore counters by construction.

Coverage (full-DB regression): REGRESSED(engine)=0. GAINED=1 (Esper Terra). Eight
counter-target fingerprint changes, all correct:
  - 4 anaphor target corrections (SelfRef→LastCreated): Esper Terra (x3 chapters, the
    +1 GAINED); applied geometry; the bus runner (first put; card stays unsupported on
    a separate gap); journey to the lost city.
  - 4 Gap-A parser wins (target unchanged SelfRef, a new "up to X" node parses):
    clockwork avian/beast/steed/swarm (stay unsupported on the counter-cap gap).
  - 9 genuine-source cards ("...on this creature/enchantment/<name>") byte-identical.

Measured blast radius (replaces the plan's 6/7 prediction): 3 anaphor cards corrected
(applied geometry, the bus runner, journey to the lost city) / 5 pre-existing-unchanged
/ 9 genuine-source preserved. The 5 unchanged cards (alien invasion, intrude on the
mind, lasting fayth, match the odds, kianne) put counters via a dynamic "for each"-count
PutCounter on a separate parser path that never reaches the it-branch — they stay
SelfRef (misbound to source), byte-identical to baseline. That path is a pre-existing,
LIVE rules-wrong gap this change does not touch; deferred and logged
(.planning/coverage-analysis/S25-DEFERRALS.md D1).

Ship-prep note: journey to the lost city appears in #5072's R3 member-bound
classification; its PutCounter target change alters its ability AST, so its
reads_member_bound rw-profile — and thus se_member_bound_class — may move +/-1 at the
next full-DB sweep. Attributed to this commit (expected movement, not a regression).

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): Foraging Wickermaw — "this creature becomes that color"

Closes the last gap on Foraging Wickermaw: "{1}: Add one mana of any color.
This creature becomes that color until end of turn." reuses the existing
chosen-color carrier — no new engine variant (empty types/ diff).

- Parser (oracle_effect/subject.rs): fold "that color" into the existing
  AddChosenColor arm of try_parse_become_color_modification, beside "the chosen
  color". "That color" is the color of the mana produced this activation — a
  resolution-time anaphor, not a value baked at parse (CR 106.1a/202.2/105.3).
- Runtime (game/mana_abilities.rs): record the produced color as
  ChosenAttribute::Color on the source in produce_mana_from_ability, so the
  Layer-5 AddChosenColor reader (already powering Puca's Eye) sets the creature's
  color (CR 613.1e). Reuses the existing mana_sources::mana_type_to_color
  (Colorless→None) — no new converter, no types/mana.rs edit.

Two safety properties, both proven by revert-to-red:
- Gated write, zero blast radius. produce_mana_from_ability is the universal
  mana chokepoint, so the write fires only when the resolving ability's own chain
  carries a downstream AddChosenColor (via visit_links_any on the activated
  ability_def, fresh per activation). Coverage-regression confirms exactly one
  card flips (Foraging Wickermaw) and every ordinary producer — Birds of Paradise,
  City of Brass, painland, filter land, basic, rock — is byte-identical.
- Explicit replace, not accumulate (CR 400.7). The stored ChosenAttribute::Color
  PERSISTS across turns (cleared only on zone change), UEOT expiry removes the
  Layer-5 effect but NOT the stored attribute, and the choice binder accumulates
  rather than replaces — so a plain push would leave a stale prior-turn color
  first in the list (chosen_color() is first-match). The write retain-drops the
  prior Color before pushing, so re-activating on a later turn with a new color
  replaces cleanly.

The Colorless→White fallback converter (mana_payment.rs) is left untouched: it is
load-bearing-defensive for the three Phyrexian-shard sites, where CR 107.4a makes
Colorless unreachable.

Assisted-by: ClaudeCode:claude-opus-4.8

* feat(engine): The Tomb of Aclazotz — cast-from-graveyard type-grant rider

Closes the last gap on The Tomb of Aclazotz's "{T}: You may cast a creature
spell from your graveyard this turn. If you do, it enters with a finality
counter on it and is a Vampire in addition to its other types." The finality
counter half was already wired via the ExileWithAltCost permission channel; this
adds the parallel channel for the "is a Vampire in addition to its other types"
type grant.

- New Effect::AddPendingEntersModifications { modifications: Vec<ContinuousModification> }
  — the general "enters-with continuous modifications" carrier (CR 613 layer
  system), a categorical sibling of AddPendingETBCounters (CR 122 physical
  counters). The Vec<ContinuousModification> shape absorbs every future
  enters-with rider (added types, granted abilities, colors), so no third sibling
  is needed. The two channels stay separate because a counter is not a continuous
  modification.
- Parser (oracle_effect/mod.rs): try_parse_cast_this_way_enters_rider lifts the
  former trailing-tail rejection (CR 205.1b) to accept "… and is a <type> in
  addition to its other types", parsing the tail via
  animation::parse_becomes_type_modifications (additive AddType/AddSubtype/
  AddSupertype — retains printed types, CR 205.1b) and emitting the new effect as
  the counter rider's sub-ability.
- Permission channel (mirrors enters_with_counter exactly): a new
  ExileWithAltCost.enters_with_modifications field (serde default + skip-if-empty),
  lifted from the rider at grant time and read back from the SELECTED permission at
  cast-finalize (CR 611.2c — no sibling-permission leak), applied to the cast
  object as a Duration::Permanent continuous effect (Layer 4, CR 613.1d).
- #5072 read/write dual-walk: AddPendingEntersModifications is classified as a
  self-scoped SetMembership write (mirroring Effect::Animate/Endure's type-line
  writes) — NOT an ObjectCounters write; …
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Larger-scoped feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants