Fix Crabomination - #10412

Summary

Crabomination's enters trigger ("target opponent exiles the top card of their library, a card at random from their graveyard, and a card at random from their hand. You may cast a spell from among cards exiled this way without paying its mana cost") parsed only its first object (ExileTop). The graveyard and hand cards were silently dropped, so the free cast could only see the library card. This PR parses the whole <subject> exiles <object list> instruction. It lowers it to one ordered chain:

• the subject's player target;
• one ChooseFromZone leg per object, each reading that player through the existing chosen-group link;
• a single ChangeZoneAll over the gathered set, so the three cards are exiled in one action (CR 608.2f) and "one or more cards are exiled" triggers fire once;
• the existing free-cast window.

A new optional typed gather_role (Opens / Joins) on ChooseFromZone scopes the gathered set to this instruction. The top-of-library object is a new ZoneChoiceCandidateSource::LibraryTop: no prompt, no random draw.

Files changed

• crates/engine/src/types/ability.rs: ZoneChoiceCandidateSource::LibraryTop; ZoneChoiceGatherRole and the optional ChooseFromZone.gather_role field (serde default + skip).
• crates/engine/src/game/effects/choose_from_zone.rs:
  • LibraryTop settle path (resolve_library_top, library_top_shape);
  • role-aware publication on the random single-owner Direct path;
  • fail-closed refusal of a role on any other path;
  • CR 608.2b empty pool for an illegal subject;
  • unit tests.
• crates/engine/src/game/ability_rw.rs, crates/engine/src/game/ability_scan.rs: classify LibraryTop; document why gather_role is not read.
• crates/engine/src/parser/oracle_effect/imperative.rs:
  • parse_subject_exile_object_list and its object grammar;
  • shared parse_exile_top_count, which delegates to parse_quantity_expr_number; parse_exile_ast now uses it too.
• crates/engine/src/parser/oracle_effect/mod.rs: lower_subject_exile_objects in the subject-predicate lowering (mints the subject's player group, assigns roles by position). A single-object subject exile whose owner is not the subject is now refused (Effect::unimplemented) instead of losing the owner.
• crates/engine/src/parser/oracle_ir/ast.rs: SubjectExileObjectsAst { first, rest } and SubjectExileObjectAst.
• crates/engine/src/parser/oracle_ir/effect_chain.rs: ClauseIrBuilder::push does not set declares_chosen_clause when the clause carries a minted declared_player_group.
• crates/engine/src/parser/oracle_effect/tests.rs: parser shape tests.
• crates/engine/tests/integration/crabomination_etb_exile_list.rs (new, 17 tests) and crates/engine/tests/integration/main.rs.
• crates/engine/tests/fixtures/integration_cards.json.gz: only the crabomination entry differs from main.
• protocol/changes/18241cfc-6cc1-4485-8f9b-c8abd54a50c9.json: game protocol bump (surfaces full + wire).
• Mechanical gather_role: None on existing ChooseFromZone literals only:
  • crates/engine/src/game/effects/{choose_one_of,exile_from_top_until,mod,perpetual,vote}.rs
  • crates/engine/src/parser/oracle_effect/{lower,sequence}.rs
  • crates/engine/src/parser/{oracle_trigger_tests,swallow_check}.rs
  • crates/engine/src/types/resolution.rs
  • crates/engine/tests/integration/{interaction_contract,issue_1374_atraxa_grand_unifier}.rs
  • crates/phase-ai/src/policies/mill_targeting.rs, crates/phase-ai/src/search.rs

Track

Developer

LLM

Model: claude-opus-5-5
Tier: Frontier
Thinking: high

Implementation method (required)

Method: /engine-implementer

CR references

Annotations added or touched:

Each number was checked against docs/MagicCompRules.txt.

Verification

• Required checks ran clean, or the exact CI-owned alternative is stated below.
• Gate A output below is for the current committed head.
• Final review-impl below is clean for the current committed head.
• Both anchors cite existing analogous code at the same seam.

All results below are at the PR head 73f68893bcec290e6e8f8d084a12a8a3db3e635a. They were run in a clean detached worktree, with tracked files clean before and after, and the full client/public/card-data.json regenerated by this head's oracle-gen.

• cargo fmt --all -- --check: exit 0
• cargo clippy --locked --workspace --exclude phase-tauri --all-targets --features engine/proptest -- -D warnings: exit 0
• node scripts/protocol-versions.mjs generate && node scripts/protocol-versions.mjs check: exit 0
• node scripts/check-protocol-version.mjs --base c2fbb7a6634a440c7c6b47e6a87a554cc7382f05: exit 0
• ./scripts/check-interaction-bindings.sh --check: exit 0
• cargo nextest run --profile ci -p phase-engine --features proptest --no-fail-fast: 39620 run, 39619 passed, 1 failed, 19 skipped. All 17 crabomination_etb_exile_list tests pass.
  • The failure is pre-existing and local-only: game::casting::tests::snuff_out_from_card_database_alt_cost_destroys_target panics on the debug assertion "a spell trigger carrier outlived its action" (game/announcement_triggers.rs:336).
  • It needs client/public/card-data.json and self-skips without it.
  • It fails identically on main c2fbb7a66 with the same freshly generated card data.
• cargo build -p probe-pin && cargo build -p phase-ai --bin ai-commander --bin ai-duel --bin ai-gate, then cargo nextest run --profile ci -p phase-ai --no-fail-fast: 3180 passed, 0 failed.
• FORGE_TEST_FULL_DB=1 cargo nextest run -p phase-engine --features proptest -E 'test(ordering_parity_sweep)': fails, pre-existing. Its stale DOCUMENTED_OVER_PROMPT entries are not hit under a fresh full DB, and it fails identically on main (same sets, same sweep stats). This is an opt-in local sweep.
• Card data: oracle-gen data/ --stats against MTGJSON 2026-10-28 (36159 faces), compared per card with main's export. Only crabomination (triggers) differs.
• Coverage: coverage-report data/ --all reports 32167 supported on main and on this head, and only Crabomination's row changes (supported, gap 0).
• Semantic audit: oracle-gen semantic-audit data/ flags 192 cards on main and on this head, with identical findings. Crabomination has none.
• Parser measurement: the base-built coverage-parse-diff reports 1 card and 2 signatures: Crabomination, +TargetOnly (target=opponent), -ExileTop. It gives the same result on MTGJSON 2026-08-09 (35809 keys) and 2026-10-28 (36159 keys), and the per-card JSON diff on both shows only crabomination.
• cargo check --release --bin card-data-validate: exit 0.
• cargo run --profile tool --bin card-data-validate -- data/card-data.json: "OK: 36159 cards parsed".
• ./scripts/coverage-regression-check.sh https://data.phase-rs.dev/preview/coverage-data.json data/coverage-data.json --fail-on-engine: exit 0, with 0 engine regressions and 0 coverage-honesty regressions.
  • Two items are baseline drift against the published preview, not changes from this PR: one oracle-rewording flip (Joven and Chandler) and one gain (Artist Alley).
  • The per-card diff against main above shows only Crabomination.
• pnpm lint / pnpm run type-check: not run locally, left to CI. This PR changes no client/ file.
• Commit hooks: the pre-commit gates were run by hand against the upstream base for every commit: parser combinator gate, PreLowered ratchet and CR citation anchors, all PASS.
  • The pre-push steps were covered by the results above.
  • Commits and the push used --no-verify because this fork's origin/main has diverged from upstream, so the hooks pick the wrong base.

Review history.

Rounds 1–3 of the final-PR loop each found behavior issues, and each was fixed with code:

1. Shared grammars; a deterministic library leg.
2. Legs bound to the subject's own target; the gather scoped to the instruction.
3. The typed gather role; the illegal-subject case; an any-order object list.

Round 4 went past AI-CONTRIBUTOR §5's retry limit, with the contributor's explicit approval.

In round 4, at this head, the read-only review-engine-impl was clean apart from residuals. The independent fresh-context cross-check (AI-CONTRIBUTOR §5 check 3) reported 4 findings, none a correctness bug against the card text or the CR. All four are listed under Validation Failures, including one MED design point that is disputed rather than fixed. The contributor decided to open the PR with it disclosed.

Pipeline-reviewed head: 1fdbc08cc0ce2152fde387da597bd0ef346219c2
Correction commit: none
Current branch head: 73f68893bcec290e6e8f8d084a12a8a3db3e635a
Pipeline status: historical — final-PR review fix rounds, fixture refreshes and two clippy-lint commits followed acceptance
Current-head review: clean at 73f68893bcec290e6e8f8d084a12a8a3db3e635a

Gate A

Gate A PASS head=73f68893bcec290e6e8f8d084a12a8a3db3e635a base=c2fbb7a6634a440c7c6b47e6a87a554cc7382f05

Anchored on

• crates/engine/src/types/ability.rs:24401: reciprocal_role: Option<ReciprocalZoneChoiceRole> on Effect::ChooseFromZone. This is an existing optional typed role on the same variant; gather_role follows its shape and serde attributes.
• crates/engine/src/parser/oracle_effect/mod.rs:23814: the damage-list lowering mints the announced player's group into ctx.clause_declared_group. The exile-list lowering uses the same seam, so its legs read the subject's own target.
• crates/engine/src/game/effects/choose_from_zone.rs:794: the random per-player pick publishes its accumulated picks as a fresh tracked set for the continuation to read. The gather roles generalize this "open a fresh set, accumulate into it" publication across one instruction's objects.
• crates/engine/src/parser/oracle_effect/imperative.rs:247: parse_library_owner_phrase, the shared library-owner grammar that the object list composes, alongside parse_exile_ast (:11390).

Final review-impl

Final review-impl PASS head=73f68893bcec290e6e8f8d084a12a8a3db3e635a

Claimed parse impact

Crabomination

Scope Expansion

• Mechanical gather_role: None additions to existing ChooseFromZone literals in 14 files outside the parser and resolver seam. These are required by the new field, and nothing else changes in those files.
• ClauseIrBuilder::push change (oracle_ir/effect_chain.rs). The rule that decides declares_chosen_clause for a TargetOnly clause with a minted player group now lives where the field is computed, instead of in a post-push fix-up in the chunk loop. Parse output is unchanged for every other card (full-corpus export).
• Single-object owner refusal. Single-object subject exiles ("target opponent exiles a card from their hand") whose possessive names a player other than the subject are now refused as Effect::unimplemented. Previously the owner was silently dropped. The PR's own claim that nothing is silently dropped is false without this fix. No card in the corpus changes parse because of it.

Validation Failures

Residual, LOW, no observed wrong behavior: a multi-object ChangeZoneAll does not record the exiling player as the move's performer. It records the ability's controller, as the existing single-object ChangeZone path does.

• Nothing reads the performer for this card class.
• The only reader today is casting.rs::exile_permission_pool's own_exiles_of, and no card of this form reaches it.
• Follow-up: record the subject as performer for both ChangeZone and ChangeZoneAll when the printed subject is another player ("target opponent exiles …").

Residual, LOW, latent, no reachable wrong behavior (final review): a reciprocal Consume ChooseFromZone pick ignores a gather_role instead of refusing it.

• Why: the refusal sits in choose_from_zone::resolve and in the random path, but a Consume pick enters resolve_with_choosing_player directly, from drive_empty_reciprocal_producer and from the producer-pick answer.
• The field doc's "refused on every other path" therefore overstates the coverage.
• Why it's unreachable:
  • The only producer of a role is lower_subject_exile_objects, which always emits reciprocal_role: None.
  • A reciprocal Produce pick that carries a role is already refused, and a unit test covers that case.
• Follow-up: move the refusal into resolve_with_choosing_player, the entry both callers share, and add the Consume case to gather_role_off_its_two_paths_is_refused_without_a_prompt_or_draw.

Disputed design point, MED from the cross-check; not changed, for maintainer judgement. ZoneChoiceCandidateSource::LibraryTop puts "the top N cards, taken whole, nobody chooses" on the candidate-provenance axis.

• What the cross-check objects to:
  • The leg's selection (Chosen) and chooser (Controller) are documented inert placeholders, pinned by a fail-closed runtime guard (library_top_shape).
  • gather_role is honored on only two paths.
• Its proposed fix: a new CardSelectionMode value meaning "take the whole pool, no chooser".
• Why it was not adopted (this point has come up in three review rounds):
  • CardSelectionMode is shared by Discard (two variants), RevealHand and CreateTokenCopyFromPool, as well as ChooseFromZone, and it serializes through the random bool compat adapter.
  • A whole-pool value would be meaningless for those effects, so it would move the representable invalid state into four other effects rather than remove it.
  • Neither ZoneChoiceChooser nor CardSelectionMode has an existing "no player chooses" value. CR 401.2 (the library's order fixes which cards are taken) is the reason the rule sits on provenance.
• If you prefer a dedicated selection type for ChooseFromZone, that's a follow-up refactor.

Residuals from the round-4 cross-check, all LOW with no reachable wrong behavior:

• ability_rw read profile: it classifies LibraryTop as a SetMembership read only. It does not also declare the library-order read (StateKind::HandLibrary), as StaticCondition::TopOfLibraryMatches does. This is not a regression: main's ExileTop lowering declared no library read either. Follow-up: merge reads_player_of(StateKind::HandLibrary) into the LibraryTop arm.
• Per-player refusal (choose_from_zone.rs): the random per-player path now treats a PerPlayerSpec::of error as empty pools instead of panicking. On the refused gather-role shape, an Opens leg does not open a fresh set. No parser output reaches either case. Follow-up: refuse only the LibraryTop shape explicitly, keep the expect for other errors, and publish an empty Opens set on refusal.
• Test assertion: in subject_exile_list_with_an_illegal_subject_gathers_nothing_from_an_earlier_exile, the "earlier card not offered" check sits inside if let WaitingFor::CastOffer. Its reach-guards are unconditional: one move of the earlier card, the leg resolutions, and the ability finished. Follow-up: assert the final waiting state unconditionally.

CI Failures

None.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added support for exiling the top card of a library alongside randomly selected cards from a hand or graveyard, then offering an eligible exiled spell for a free cast.
  • Multiple selections follow the instruction’s listed order and apply to the specified player’s zones. Selected cards are exiled together in one action.
• Bug Fixes
  • Empty or unsupported selection pools now resolve safely without drawing cards, and unrelated previously tracked cards are excluded from the exile action.
  • Free-cast offers exclude lands and are limited to one eligible spell.