feat(engine): coerced-attack + end-step punisher (Siren's Call, Maddening Imp)

[Spaceint]

Summary

Implements the coerced-attack + end-step punisher class — "creatures the active player controls attack this turn if able, then destroy the ones that didn't at the next end step":

• Siren's Call — "Cast this spell only during an opponent's turn, before attackers are declared. Creatures the active player controls attack this turn if able. At the beginning of the next end step, destroy all non-Wall creatures that player controls that didn't attack this turn. Ignore this effect for each creature the player didn't control continuously since the beginning of the turn."
• Maddening Imp — "Flying. {T}: Non-Wall creatures the active player controls attack this turn if able. At the beginning of the next end step, destroy each of those creatures that didn't attack this turn. Activate only during an opponent's turn and only before combat."

Builds on #5192 ("make the 'before attackers declared' window phase-based", already merged) — Maddening Imp's activation gate (and Siren's Call's cast window) require that fix to be reachable by a non-active actor. This PR is rebased on current main and stands alone.

Engine primitives added (2 new variants, both gated via /add-engine-variant)

• ControllerRef::ActivePlayer (CR 102.1) — the turn player, resolved live from state.active_player. Distinct from Opponent (which matches every opponent in multiplayer); a leaf sibling of DefendingPlayer/EnchantedPlayer/TriggeringPlayer. Threaded through the exhaustive ControllerRef match sweep.
• FilterProp::ControlledContinuouslySinceTurnBegan (CR 302.6 / 508.1a) — haste-independent control-continuity predicate (raw summoning_sick), distinct from the haste-coupled HasHasteOrControlledSinceTurnBegan. Powers Siren's Call's "didn't control continuously since the turn began" exemption.

Parser

• Recognize "the active player controls/controlled" as a subject controller (parse_zone_controller + the past-tense suffix).
• A post-lowering card-assembly pass binds Siren's Call's delayed DestroyAll controller to ActivePlayer (its clause parses as a separate ability, so scope can't cross the line boundary) and re-binds Maddening Imp's "those creatures" anaphor to the concrete filter.
• Compound activation-timing recognition ("during an opponent's turn and only before combat" / ", before attackers are declared") — reuses the existing enforced RequiresCondition{Not(IsYourTurn)} + BeforeAttackersDeclared primitives (no new variant), which also fixes Nettling Imp (same compound-phrase gap) and lets a 25-line verbatim-string hack be deleted (subsumed by the general parser; 27 legacy "during your turn, before attackers" cards keep working).

Known deviation (disclosed)

Maddening Imp "those creatures" is re-bound to a live filter Typed{Creature, Non(Wall), controller:ActivePlayer, Not(AttackedThisTurn)} rather than a frozen population snapshotted at resolution (there is no tracked-set producer for forced attackers). This diverges from CR 608.2c in three narrow directions — late-arrival over-inclusion, control-change-out under-inclusion, left-and-re-entered re-capture — all documented in-code and pinned by a test. A faithful frozen-population producer is a single-swap-site follow-up I'm happy to do if preferred.

Anchored on

• crates/engine/src/types/ability.rs — ControllerRef::EnchantedPlayer/DefendingPlayer: the game-defined-player-role family the new ActivePlayer variant joins.
• crates/engine/src/game/filter.rs — controller_ref_player (exhaustive, no wildcard): the single resolution seam extended with ActivePlayer => Some(state.active_player).
• crates/engine/src/parser/oracle.rs — synthesize_etb_exile_ltb_return_pair: the post-lowering cross-ability reconciliation pass the new bind_active_player_punisher_target pass mirrors.
• crates/engine/src/parser/oracle_nom/filter.rs — parse_zone_controller: the controller-suffix combinator extended with the ActivePlayer arm.

Test Plan

New crates/engine/tests/coerced_attack_punisher.rs (17 tests) + one in-crate unit test. Drives the real parse + filter/resolution runtime; every negative has a positive reach-guard and fails on revert of its fix. Highlights:

• 3-player runtime: Siren's Call cast during opponent A's turn forces A's creatures (not B's or the caster's), then at end step destroys A's non-attacker while sparing the attacker and B's creature.
• ControllerRef::ActivePlayer resolves live; Opponent discriminator matches multiple players in 3-player.
• ControlledContinuouslySinceTurnBegan is haste-independent (discriminates from the haste-coupled sibling).
• Maddening Imp activation legality across own-turn (illegal) / opponent precombat (legal) / opponent post-attackers (illegal), with the activating player realistically holding priority (works via the companion window fix).
• Lavinia non-regression + Nettling Imp fixed-for-free + verbatim-hack subsumption.

Verification (Non-developer track; Tilt down → direct cargo)

• cargo test -p engine — 203 test binaries, 0 failures (incl. 17 new), rebased on current main
• CI-parity clippy — exit 0
• cargo check -p engine-wasm --target wasm32-unknown-unknown — exit 0
• cargo check -p mtgish-import — exit 0 (the one additive strict-fail ControllerRef::ActivePlayer arm; no mtgish logic)
• cargo fmt --all — clean
• Gate A — exit 0

Pre-existing, unrelated: cargo check -p engine --features forge fails with 16 errors (ManaProduction.contribution, Effect missing fields, SacrificeCost, …) — verified pre-existing on upstream/main via a clean-base build; this diff touches zero forge files (forge uses string-keyed construction, no exhaustive ControllerRef/FilterProp match).

Model: claude-opus-4-8
Thinking: high
Tier: Frontier
Track: Non-developer (card-data.json integration deferred to CI per docs/AI-CONTRIBUTOR.md §2)

[gemini-code-assist]

Code Review

This pull request implements support for the active player role (ControllerRef::ActivePlayer) and the control continuity predicate (FilterProp::ControlledContinuouslySinceTurnBegan) to correctly handle cards like Siren's Call and Maddening Imp, along with adding comprehensive tests. The reviewer feedback highlights opportunities to improve composability by avoiding hardcoded "Wall" subtype restrictions in general lowering helpers, and to ensure strict rule fidelity by not restricting control continuity checks solely to creatures. Additionally, the reviewer notes that several rules-touching boilerplate match arms are missing mandatory CR annotations.

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.

[gemini-code-assist]

[MEDIUM] Ensure all rules-touching boilerplate match arms carry CR annotations. Evidence: crates/engine/src/game/ability_rw.rs:2334.
Why it matters: Every rules-touching line of engine code must carry a verified CR annotation to maintain strict fidelity and auditability of rule implementations (this also applies to line 2579).
Suggested fix: Add a comment referencing CR 302.6.

Suggested change

	| FilterProp::ControlledContinuouslySinceTurnBegan
	// CR 302.6: control continuity.
	| FilterProp::ControlledContinuouslySinceTurnBegan

References

1. CR annotations are mandatory and verified. Every rules-touching line of engine code must carry a comment of the form CR : . ^(link)

[Spaceint]

Left as-is. These arms (ability_rw.rs:2334/2579) are RwProfile plumbing — they group the variant with the other temporal FilterProps (EnteredThisTurn, AttackedThisTurn, ZoneChangedThisTurn, …) for read/write classification, not rule implementation. Per CLAUDE.md ("Do not annotate boilerplate, serialization, or plumbing — only code that implements a game rule"), none of the sibling variants in these arms are CR-annotated; the rule-implementing eval arm in filter.rs carries the CR 302.6 / 508.1a annotation.

🤖 Addressed by Claude Code

[gemini-code-assist]

[MEDIUM] Ensure all rules-touching boilerplate match arms carry CR annotations. Evidence: crates/engine/src/game/filter.rs:216.
Why it matters: Every rules-touching line of engine code must carry a verified CR annotation to maintain strict fidelity and auditability of rule implementations (this also applies to lines 440, 3132, and 4629).
Suggested fix: Add a comment referencing CR 302.6.

Suggested change

	| FilterProp::ControlledContinuouslySinceTurnBegan
	// CR 302.6: control continuity.
	| FilterProp::ControlledContinuouslySinceTurnBegan

References

1. CR annotations are mandatory and verified. Every rules-touching line of engine code must carry a comment of the form CR : . ^(link)

[Spaceint]

Left as-is — same rationale. filter.rs:216/440/3132/4629 are structural grouping arms (the variant listed alongside EnteredThisTurn/AttackedThisTurn/etc. for filterability classification), not rule logic; per CLAUDE.md these plumbing arms are exempt from CR annotation and no sibling variant in them is annotated. The rule-implementing eval arm is annotated (CR 302.6 + 508.1a).

🤖 Addressed by Claude Code

[matthewevans]

[HIGH] The Maddening Imp delayed punisher live-refilters instead of using the objects named by “those creatures.” Evidence: crates/engine/src/parser/oracle.rs:2439 documents the TrackedSet(0) rewrite as a live-refilter deviation, and crates/engine/tests/coerced_attack_punisher.rs:286 asserts that a late-arriving creature is matched even though it was never in the original coerced set. Why it matters: CR 608.2c requires the effect to keep referring to the objects identified as “those creatures” when the ability resolved; this implementation destroys late arrivals, spares control-change-outs, and can re-capture new objects after zone changes. Suggested fix: publish a real tracked set/snapshot when the mass MustAttack effect resolves and make the delayed DestroyAll consume that tracked population, applying the non-Wall and did-not-attack predicates without reselecting a fresh population.

[MED] The Siren’s Call runtime test bypasses the cast path even though the restriction it cites is now phase-only and should admit a non-active caster. Evidence: crates/engine/tests/coerced_attack_punisher.rs:580 says cast().resolve() is unreachable because BeforeAttackersDeclared requires the priority seat to be active, but crates/engine/src/game/restrictions.rs:1925 now makes that gate a pure phase check and crates/engine/src/game/restrictions.rs:4162 tests non-active casters. Why it matters: this PR claims card support and changes casting/activation restrictions, but the main runtime test skips the production casting pipeline that would prove the parsed spell can actually be cast and resolved legally. Suggested fix: drive Siren’s Call through GameRunner::cast(...).resolve() on an opponent’s precombat/begin-combat priority window, or remove the stale premise and add the missing front-door coverage for the remaining blocker.

I also could not approve this while the required parse-diff evidence is absent for the current engine/parser head; the current checks still show the card-data/coverage job pending, so the sticky parse-diff may simply not have posted yet.

[github-actions]

Parse changes introduced by this PR · 3 card(s), 5 signature(s) (baseline: main 0b1bf20442b2)

1 card(s) · ability/DamageAll · field filter: tracked set #0 → tracked set #0 matching with Flying creature

Examples: Winter Blast

1 card(s) · ability/MustAttack · added: MustAttack (affects=the active player controls creature non-Wall, duration=until end of turn, grants=MustAttack, kind=activated)

Examples: Maddening Imp

1 card(s) · ability/MustAttack · added: MustAttack (affects=the active player controls creature, duration=until end of turn, grants=MustAttack)

Examples: Siren's Call

1 card(s) · ability/creatures · removed: creatures

Examples: Siren's Call

1 card(s) · ability/non-wall · removed: non-wall (kind=activated)

Examples: Maddening Imp

1 card(s) had Oracle-text changes (errata/reprint) — excluded as non-parser.

[matthewevans]

[HIGH] Maddening Imp still live-refilters “those creatures” instead of snapshotting the objects named as the ability resolves. Evidence: crates/engine/src/parser/oracle.rs:2459 documents the rebound punisher as live-refiltered, and crates/engine/tests/coerced_attack_punisher.rs:307 asserts a late-arriving creature is matched even though it was never among “those creatures.” Why it matters: CR 603.7c / CR 608.2c require the delayed trigger to keep referring to the objects named by the resolving ability, so this will over-include late arrivals, under-include creatures that changed controller, and re-capture new objects after zone changes. Suggested fix: create a snapshot/tracked-set producer for the MustAttack affected objects at ability resolution, then have the delayed DestroyAll consume that frozen set with the “didn’t attack this turn” filter at end step.

[Spaceint]

Both review findings addressed in b6a7308 (rebased on current main).

[HIGH] — frozen "those creatures" snapshot. Maddening Imp no longer live-refilters. The mass MustAttack coerce now publishes a frozen tracked set of its affected population when it resolves (a filter-driven GenericEffect arm in affected_objects_from_events, gated by the existing next_sub_needs_tracked_set publish gate so a standalone mass-MustAttack never publishes), and the delayed DestroyAll consumes it as TrackedSetFiltered { filter: Not(AttackedThisTurn) }. Crucially, the sentinel TrackedSetId(0) is now pinned to the concrete id at delayed-trigger creation — a new DestroyAll arm in bind_tracked_set_to_effect (reusing the existing TargetFilter::rebind_tracked_set_sentinel) — so end-step resolution reads the frozen set instead of a live max_by_key scan that a later-published tracked set could hijack. Correct on all three CR 608.2c / 603.7c / 400.7 cases: late arrivals excluded, control-change-outs still destroyed (same ObjectId), re-entered objects excluded. The old live-refilter route and its 3-direction deviation note are deleted. New runtime tests: late-arrival spared, control-change-out destroyed, and a cross-resolution collision test proving a higher-id tracked set published between {T} and end step does not hijack the punisher (fails without the sentinel pin). Siren's Call is unaffected — it re-specifies the population explicitly, not via a "those creatures" anaphor.

[MED] — Siren's Call real cast path. The Test A premise was stale: since #5192 made the before-attackers window phase-only, a non-active caster can cast it front-door. Test A now drives the real GameRunner::cast(...).resolve() on an opponent's pre-combat priority window and asserts the coerce + end-step punisher through the production pipeline.

Verified on the current-main merge: full cargo test -p engine (204 bins, 0 fail), CI-parity clippy, engine-wasm, mtgish-import, and Gate A all green; parse-diff remains isolated to Siren's Call + Maddening Imp.

🤖 Addressed by Claude Code

[matthewevans]

[LOW] The branch includes unrelated generated changelog state. Evidence: client/public/changelog.json:1, client/public/changelog-meta.json:2, and scripts/changelog/state.json:2 add release id 165 for “Morph face-down casting arrives,” which is unrelated to the coerced-attack punisher engine/parser work. Why it matters: this PR should only ship the Siren's Call / Maddening Imp class; carrying release metadata can publish or overwrite changelog state independently of the release process. Suggested fix: remove the changelog files from this branch.

[Spaceint]

[LOW] changelog files — already resolved at the current head (b6a7308).

The client/public/changelog.json / changelog-meta.json / scripts/changelog/state.json entries leaked in transiently via a routine git merge upstream/main (relative to the older merge-base at the time of your review). The latest merge synced them byte-identical to main, so they are no longer in the PR diff:

• git diff main...HEAD -- 'client/public/changelog*' scripts/changelog/state.json → empty.
• GitHub's Files-changed for this PR is 25 files, all under crates/engine/** + crates/mtgish-import/** — no changelog files, nothing else unrelated.

So there's nothing to remove (deleting them would instead propose removing main's changelog state), and the squash-merge won't touch changelog metadata. The branch now carries only the Siren's Call / Maddening Imp engine + parser class.

🤖 Addressed by Claude Code

[matthewevans]

Maintainer sign-off: current-head review clean; proof, parse-diff, runtime tests, CI, and queue gates verified.