fix(engine): scope "attacks the monarch" triggers + bind destroy target to the monarch (The Spear of Bashenga #5249)

[shin-core]

Fixes #5249 (claimed there). Verified The Spear of Bashenga's Oracle text on Scryfall and reproduced end-to-end before building.

The bug

"Whenever equipped creature attacks the monarch, destroy target tapped nonland permanent that player controls." The parser's parse_attack_target had no " the monarch" arm, so the trigger degraded to a bare TriggerMode::Attacks with no attack-target scope — it fired on every attack and prompted for a destroy target regardless of who was the monarch.

Two-part fix (both class-level building blocks)

1. AttackTargetFilter::Monarch — a Player-type attack whose defending player must currently hold the monarch designation. The monarch is a dynamic single-player identity (CR 725.1) the pure type matcher can't see, so attack_target_type_matches treats Monarch as a Player attack and the identity check is applied statefully in attack_target_matches against state.monarch (both Some and equal; no monarch → never fires). Parser arm tag(" the monarch").

2. The destroy target was mis-bound — a wrong-player-target bug the fix caught. "attacks the monarch ... that player controls" parsed with controller: You — the Spear's controller, i.e. the attacker — so the destroy would have hit the attacker's own tapped permanents, strictly worse than the original bug. Root cause: parse_referenced_player_phrase (the antecedent binder feeding DefendingPlayer) had no "the monarch" arm, so "that player" never bound to the defender. Added tag("the monarch") there so "that player" binds to ControllerRef::DefendingPlayer (the monarch is the attacked player, CR 508.1a + CR 725.1) — also covers "deals combat damage to the monarch ... that player" for the whole class.

AttackTargetFilter::Monarch is additive; every exhaustive match carries an explicit arm (no wildcards). CR annotations (508.1a, 725.1) grep-verified.

Tests (each revert-probed)

• parser shape: attack_target_filter = Monarch, valid_card = the equipped-creature subject, and the destroy target is controlled by DefendingPlayer (guards the wrong-target fix), zero Unimplemented.
• matcher unit test: monarch defender → matches; non-monarch → no; no monarch → no.
• integration (real combat pipeline, 3 players): attacking the monarch fires the trigger; attacking a non-monarch does NOT fire (the reported bug — I confirmed that disabling the stateful check makes both the non-monarch and no-monarch tests fire the trigger, reproducing the exact symptom); no monarch → no fire.
• regressions: Curse "attacks enchanted player" + 3 Curse of Predation tests confirm the parse_referenced_player_phrase change causes no antecedent-binding collateral.

Verification

Full engine lib: 17065 passed, 0 failed. Integration: 3412 passed, 0 failed. Clippy clean; fmt clean; parser combinator gate PASS.

🤖 Generated with Claude Code

[superagent-security]

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

[gemini-code-assist]

Code Review

This pull request implements support for the 'attacks the monarch' trigger condition (Issue #5249, e.g., for 'The Spear of Bashenga'). It introduces the AttackTargetFilter::Monarch enum variant, updates the parser to recognize 'the monarch' as an attack target and reference player, implements the stateful runtime check in attack_target_matches against state.monarch, and adds comprehensive unit and integration tests. I have no feedback to provide as the implementation is highly idiomatic, conforms to the MTG Comprehensive Rules, and includes thorough test coverage.

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.

[matthewevans]

Authorized maintainer handling attempted to bring 19c78a7ddc280d44d2119b971ab75dcc79319d75 current in an isolated worktree.

The only textual conflict is the rustfmt-sorted module registry at crates/engine/tests/integration/main.rs (the deterministic test-registration collision). I aborted and released the assignment rather than push an unvalidated port: the active Tilt instance does not verify this worktree, and this sweep is not permitted to run direct builds. The PR is held pending a validated current-main integration.

[github-actions]

Parse changes introduced by this PR · 1 card(s), 1 signature(s) (baseline: main d6ff977fe6cb)

🟡 Modified fields (1 signature)

• 1 card · 🔄 ability/Destroy · changed field target: tapped you control permanent non-land → tapped defending player controls permanent non-land
  • Affected (first 3): The Spear of Bashenga

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

[matthewevans]

Request changes — the current head does not compile, and the required current-head engine parse-diff evidence is absent.

🔴 Blockers

1. [HIGH] The new matcher unit test passes the wrong argument type. attack_target_matches requires &TriggerSourceContext, but all three calls pass source_id: ObjectId (crates/engine/src/game/trigger_matchers.rs:5020-5025, 5032-5037, 5046-5051; CI E0308). Use the existing test_trigger_source_context(&state, source_id) helper at :4924-4945 and pass a reference to that context. The Rust lint and both Rust test shards fail on this exact error.

2. [HIGH] This engine/parser PR has no current parse-diff artifact. The PR has no <!-- coverage-parse-diff --> comment despite changed trigger/parser sources and CI having run. Regenerate the artifact for this head and account for its card-level changes before the PR can be approved.

Gemini's clean review applies only to the superseded 19c78a7 head; it does not cover this failing head. I did not run local builds.

Recommendation: fix the compile error, regenerate current-head parse evidence, and request re-review.