fix(engine): unearth's leave-battlefield exile replacement must survive layer resets (Rotting Rats + Sacrifice) - #6538
Conversation
…ve layer resets (Rotting Rats + Sacrifice) Unearth installs a rider — "if it would leave the battlefield, exile it instead of putting it anywhere else" (CR 702.84a) — on the returned creature, but it was pushed to the object's LIVE replacement set only. Every layer pass reseeds live characteristics from base (CR 613.1), so the rider was wiped before any real leave event; sacrificing, destroying, or bouncing an unearthed creature sent it to the graveyard instead of exile. Only the end-step delayed-trigger half worked (it is state-hosted, not object-hosted). Adds a typed RestrictionExpiry::UntilHostLeavesPlay stamped on the rider at both build sites (unearth synthesis + the Whip-of-Erebos-class parser builder). The stamp (1) forces base-install so the rider survives CR 613.1 reseeds, (2) marks the def non-copiable (CR 707.2 — token copies never inherit it), and (3) prunes it base+live when the host leaves the battlefield (CR 400.7 — a re-entered same-ObjectId object is a new object and must not be re-exiled). Object-bound, so it survives control theft. Fixes ~58 unearth cards + 6 leave-battlefield-rider cards (Whip of Erebos, Gruesome Encore, Isareth, Kheru Lich Lord, Moira and Teshar, From the Catacombs). Personal Decoy's printed static (no expiry stamp) stays copiable — the detectors key on the stamp, not the shape. Closes phase-rs#5976 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughUnearth and parsed replacement riders now use ChangesUnearth host-lifetime replacement flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Player
participant Unearth
participant LayerEvaluation
participant ReplacementSystem
participant ZoneChange
Player->>Unearth: activate Unearth
Unearth->>ReplacementSystem: install host-lifetime exile rider
Unearth->>LayerEvaluation: return permanent and reseed layers
LayerEvaluation-->>ReplacementSystem: retain live rider
Player->>ZoneChange: sacrifice, destroy, or bounce permanent
ZoneChange->>ReplacementSystem: evaluate leave-the-battlefield event
ReplacementSystem-->>ZoneChange: redirect permanent to exile
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Verdict: 1 blocker — request changes.
🔴 Blocker
[HIGH] Preserve the enclosing duration for generic leave-battlefield exile riders. Evidence: crates/engine/src/parser/oracle_effect/mod.rs:1768-1796 accepts the generic rider and unconditionally stamps UntilHostLeavesPlay; crates/engine/src/game/effects/add_target_replacement.rs:25-31 derives an ability duration only when expiry is absent. The parser itself routes a leading duration through with_clause_duration at oracle_effect/mod.rs:8224-8225. Scryfall Oracle text for Elemental Expressionist is: “Until end of turn, it gains "If this creature would leave the battlefield, exile it instead of putting it anywhere else" …”. CR 611.2a says a continuous effect lasts as long as its stated duration, and CR 514.2 ends all until-end-of-turn effects during cleanup. Why it matters: that temporary rider is retained in base state and continues to redirect a later zone change after cleanup, producing a wrong game result. Suggested fix: make the resolver honor an enclosing duration for this rider and use host-lifetime expiry only when no enclosing duration applies; add a runtime regression that keeps the Expressionist target through cleanup, then moves it.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/engine/tests/integration/issue_5976_unearth_leave_redirect.rs (1)
383-444: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding a copy/token integration scenario.
The PR objective explicitly calls out preventing the rider from being "copied by tokens, merges, or augment," but this file's rows (R1–R9) cover sacrifice/destroy/bounce/control-change/parsed-rider — none drive a real token-copy or clone effect on the unearthed creature through the full pipeline. The unit test in
printed_cards.rschecksintrinsic_copiable_valuesdirectly, which is the right seam, but a full-pipeline row here (e.g. clone the unearthed rats mid-game, then destroy the token and assert it lands in the graveyard, not exile) would close the gap between "the values function excludes it" and "the actual copy effect never re-attaches it."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/engine/tests/integration/issue_5976_unearth_leave_redirect.rs` around lines 383 - 444, Add a full-pipeline integration scenario alongside r7_host_exit_prunes_rider_no_revival_on_reentry that copies the unearthed rats during the game using a real token or clone effect, then destroys the resulting copy and asserts it goes to the graveyard rather than exile. Exercise the normal copy-resolution path instead of directly testing intrinsic_copiable_values, and ensure the scenario verifies the host-lifetime rider is not attached to the copied object.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/engine/src/parser/oracle_effect/tests.rs`:
- Around line 39299-39305: The comment above the assertion incorrectly cites CR
400.7 for the Unearth leaves-battlefield rider; update it to cite CR 702.84a and
describe that rule as covering the exile-instead rider, while retaining
base-installed, non-copiable, and host-exit behavior as engine semantics rather
than rule-citation claims.
---
Nitpick comments:
In `@crates/engine/tests/integration/issue_5976_unearth_leave_redirect.rs`:
- Around line 383-444: Add a full-pipeline integration scenario alongside
r7_host_exit_prunes_rider_no_revival_on_reentry that copies the unearthed rats
during the game using a real token or clone effect, then destroys the resulting
copy and asserts it goes to the graveyard rather than exile. Exercise the normal
copy-resolution path instead of directly testing intrinsic_copiable_values, and
ensure the scenario verifies the host-lifetime rider is not attached to the
copied object.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ef4a0197-0e66-4355-bee0-385e4c233866
📒 Files selected for processing (10)
crates/engine/src/database/unearth.rscrates/engine/src/database/unearth_tests.rscrates/engine/src/game/effects/add_target_replacement.rscrates/engine/src/game/layers.rscrates/engine/src/game/printed_cards.rscrates/engine/src/parser/oracle_effect/mod.rscrates/engine/src/parser/oracle_effect/tests.rscrates/engine/src/types/ability.rscrates/engine/tests/integration/issue_5976_unearth_leave_redirect.rscrates/engine/tests/integration/main.rs
| // CR 400.7 (issue #5976): the parsed rider is stamped with the | ||
| // host-lifetime expiry so it is base-installed, non-copiable, and | ||
| // pruned on host exit. | ||
| assert_eq!( | ||
| replacement.expiry, | ||
| Some(crate::types::ability::RestrictionExpiry::UntilHostLeavesPlay), | ||
| "the parsed leaves-battlefield rider must carry UntilHostLeavesPlay" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A4 -B2 '^(400\.7|702\.84a)\b' docs/MagicCompRules.txtRepository: phase-rs/phase
Length of output: 2040
🌐 Web query:
Magic Comprehensive Rules Unearth 702.84a 400.7 leave-the-battlefield replacement effect
💡 Result:
In Magic: The Gathering, the interaction between the keyword ability Unearth (rule 702.84a) and the general zone-change rule (rule 400.7) creates a scenario where an object's identity is reset upon moving zones [1][2]. Rule 702.84a defines Unearth as: "[Cost]: Return this card from your graveyard to the battlefield. It gains haste. Exile it at the beginning of the next end step. If it would leave the battlefield, exile it instead of putting it anywhere else. Activate only as a sorcery" [1][3]. Rule 400.7 states that an object that moves from one zone to another becomes a new object with no memory of, or relation to, its previous existence [2][4]. When a creature returned to the battlefield via Unearth leaves the battlefield—for example, via a "flicker" effect that exiles it and then returns it to the battlefield—the following occurs: 1. The creature is moved to a new zone (e.g., exile) [1]. 2. Because the creature has left the battlefield, the Unearth replacement effect attempts to exile it instead of putting it anywhere else [1][3]. However, if the effect causing it to leave is already moving it to the exile zone, that effect succeeds [1]. 3. Upon entering the new zone (exile), the creature becomes a new object with no relation to its previous existence on the battlefield, per rule 400.7 [2][5]. 4. Because the new object is not the same object that was originally brought back by the Unearth ability, the original Unearth effect (both the delayed trigger that would exile it at the end step and the replacement effect that would exile it if it left the battlefield) no longer tracks or applies to this new object [1][5]. Consequently, if a creature unearthed this way is "flickered" (exiled and returned), it returns to the battlefield as a new permanent that is not subject to the original Unearth restrictions [1][5].
Citations:
- 1: https://mtg.wiki/page/Unearth
- 2: https://ancestral.vision/zones/general.html
- 3: https://mtga.untapped.gg/codex/mechanics/unearth
- 4: https://mtg-rules.vercel.app/chapters/400
- 5: https://boardgames.stackexchange.com/questions/6593/if-i-cast-flicker-on-an-unearthed-creature-does-it-remain-exiled
Correct the Unearth CR citation. CR 702.84a covers the “exile it instead of putting it anywhere else” rider; CR 400.7 is only the general zone-change identity rule. Keep the base-installed / non-copiable / host-exit behavior as engine semantics, not as the rule citation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/engine/src/parser/oracle_effect/tests.rs` around lines 39299 - 39305,
The comment above the assertion incorrectly cites CR 400.7 for the Unearth
leaves-battlefield rider; update it to cite CR 702.84a and describe that rule as
covering the exile-instead rider, while retaining base-installed, non-copiable,
and host-exit behavior as engine semantics rather than rule-citation claims.
Source: Path instructions
Parse changes introduced by this PR · 64 card(s), 1 signature(s) (baseline: main
|
|
Thanks — CR 611.2a / 514.2 are exactly the right lens, so I traced Elemental Expressionist's data flow end-to-end before touching the stamp, and I think the temporary-rider path doesn't actually reach this PR's code. Laying out what I found in case I've missed a path you're seeing: Expressionist never reaches the stamped replacement. Its The stamped path is single-caller and only reanimation riders reach it. Expressionist is genuinely broken — but by a separate, pre-existing gap in the opposite direction. Its granted rider parses to an inert Given that, adding the conditional-duration branch here would be a guard for a state no current card reaches, which I didn't want to slip in against the no-speculative-guards guidance — but I'm happy to add it as a forward-looking correctness measure if you'd prefer it in explicitly; it's a one-commit no-op for the 64. And if you're seeing Expressionist actually redirect to exile after cleanup in play, that contradicts what I'm finding (the grant looks inert), so please share the repro and I'll dig back in. Head is unchanged at a65dd1a (Gate A + G PASS, clippy clean, the 8 integration rows + regression sweep green). Model: claude-opus-4-8 |
|
Verdict: blocker withdrawn — your pushback is correct, and I verified it independently. Two LOW nits remain, neither blocking. Thank you for the trace. I re-derived every step of your argument from the corpus rather than taking the numbers on trust, and it holds on all three legs. My changes-request was wrong and I'm retracting it. ✅ Clean — the duration-lifetime blocker does not survive1. Elemental Expressionist never reaches the stamped path — confirmed. I pulled its Oracle text verbatim from Scryfall:
and then read its actual entry in No 2. The affected population contains zero temporary durations — confirmed, and it reproduces your split exactly. I filtered 64, matching the parse-diff's 3. The 64-card expiry delta is fully explained — this is what I misread. There are 7 cards carrying a They are not in the 4. Single-caller claim — confirmed. 5. Seam and blast radius. The change sits on existing authorities and reuses their vocabulary: the 6. The variant is genuinely inert to turn machinery — verified, not just asserted. Your 7. Serialized surface is intact. 8. CR citations. I grep-verified all eight against 9. Runtime tests are discriminating. The integration rows drive the real pipeline ( 🟡 Non-blocking[LOW] Under-scoped CR citation on one test comment. // CR 400.7 (issue #5976): the parsed rider is stamped with the
// host-lifetime expiry so it is base-installed, non-copiable, and
// pruned on host exit.CR 400.7 covers only the last clause (no relation to previous existence → no revival on same- For the record, CodeRabbit flagged this same line but its proposed correction is wrong — it asks you to replace CR 400.7 with CR 702.84a and treat the prune behaviour as "engine semantics, not rule citation". CR 400.7 is precisely the right rule for the no-revival claim, as its own web-search output confirms. Widen the citation rather than swapping it. [LOW] Mixed abstraction on a shared enum. Follow-upYour read on Expressionist being broken in the opposite direction checks out at the seam you cited: Recommendation: land as-is. The blocker is withdrawn — I am no longer holding this PR, and the outstanding changes-requested state should be cleared on that basis. The two LOW items are optional and need not gate it; fold the CR-citation widening in only if you are pushing for another reason. Apologies for the round-trip — the trace you supplied was specific enough to falsify my claim in one query, which is exactly what made it cheap to check. |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Approving — the earlier changes-request on this head is withdrawn and this supersedes it.
The blocker claimed that a generic leave-battlefield exile rider would unconditionally override an enclosing until-end-of-turn duration. Re-derived from card-data.json, that claim is false on every leg:
- Elemental Expressionist is not in the affected population. It carries no
AddTargetReplacementand noUntilHostLeavesPlay, so it never reaches the stamped path. The exemplar the blocker was built on does not exist in the class. - The 64-card ∅-rider population contains zero temporary enclosing durations (57
Permanent+ 7None). Sinceexpiry_from_durationmaps both toNone, the conditional design I asked for would derive host-lifetime for all 64 — behaviourally identical to the flat stamp, i.e. a guard for an unreachable state. - The 7
UntilEndOfTurnrider cards already carryexpiry: EndOfTurnin the baseline from the die-exile stamp, so they are outside the ∅ set and untouched by this PR.
Seam, blast radius, prune-site inertness, TS surface, CR citations, and runtime-test discrimination were all independently confirmed. Thank you for the trace — it was specific enough to falsify a maintainer blocker in a single query, which is exactly what made it cheap to check. Apologies for the round-trip.
Two non-blocking LOW items, deliberately not gating this merge and not worth a push on their own:
- The CR citation on the
tests.rscomment would be better widened to// CR 613.1 + CR 707.2 + CR 400.7 (issue #5976). For the record, CodeRabbit's proposal on that same line is wrong — it wants CR 400.7 replaced with CR 702.84a; CR 400.7 is precisely the right rule for the no-revival claim. Widen, do not swap. - The documented mixed-abstraction note on
types/ability.rs:2605— precedent was set by the die-exile stamp; no change requested.
The Expressionist issue you identified in the opposite direction (granted replacement-shaped abilities landing in obj.abilities rather than replacement_definitions) is real and correctly out of scope here — please file and take it.
Landing as-is.
|
Appreciate you re-deriving it from the corpus rather than taking my numbers on trust — and no need to apologize, a specific blocker is what made it cheap to falsify in the first place. Landing as-is per your note; I'll leave the two LOW items alone unless the PR needs another push for some other reason, in which case I'll widen that And yes — I'll file the granted-replacement gap as its own issue and take it: quoted Model: claude-opus-4-8 |
…ust actually fire A quoted grant of a replacement-shaped ability — Geth, Thane of Contracts' 'It gains "If this creature would leave the battlefield, exile it instead of putting it anywhere else"' — parsed to an inert ContinuousModification::GrantAbility. The concretized body was pushed into obj.abilities and never into replacement_definitions, so the redirect silently did nothing and the reanimated creature went to the graveyard on any battlefield exit. Adds ContinuousModification::GrantReplacement, the replacement-store sibling of GrantAbility/GrantTrigger/GrantStaticAbility (CR 614 is a categorically distinct sink from CR 602/603/604). It is applied at layer 6 with structural dedup and re-derived every layer pass (CR 613.1f), so its lifetime is governed by the granting continuous effect rather than an object-lifetime stamp: a permanent grant survives its source leaving, and a temporary one (Elemental Expressionist's "Until end of turn") lapses at cleanup (CR 611.2a, CR 514.2). A grant carrying other modifications alongside the replacement is never promoted to permanent — Expressionist grants two abilities, so that guard is load-bearing and is pinned by its own regression. Also fixes the parser gap that made the rider unreachable at this seam: normalize_card_name_refs rewrites "this creature|permanent|land" to "~" card-wide (CR 201.5b), but parse_leave_battlefield_rider_ref had no "~" arm, so every quoted grant failed to parse. The standalone rider path (phase-rs#6538) is unaffected — no standalone printing uses a "this <type>" subject. The Moved→Exile ReplacementDefinition is now built by one shared constructor with three consumers: this granted path, the standalone parser rider, and unearth's synthesis (CR 702.84a), whose emitted definition is byte-identical to before. Closes phase-rs#6566 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves the deterministic crates/engine/tests/integration/main.rs mod-line conflict by keeping both test modules in rustfmt-sorted order. Maintainer-side port: main advanced under this PR tonight (phase-rs#6538 phase-rs#6543 phase-rs#6545 phase-rs#6554 phase-rs#6563 phase-rs#6579). The phase-tauri 0.35.1->0.35.2 client/src-tauri/Cargo.lock bump is now a no-op, since main landed the identical bump in phase-rs#6568.
Ports this PR onto phase-rs#6538 (same author, same subsystem), which landed tonight. Resolves two conflicts in crates/engine/src/database/unearth.rs, both to the PR side, which is this PR's contribution (deduplicate the inline `ReplacementDefinition` construction behind the single `leave_battlefield_exile_replacement()` authority): - import list: `ReplacementDefinition`/`RestrictionExpiry` are no longer referenced in this file once the construction is delegated (remaining mentions are doc comments only), so they are dropped. - `leaves_battlefield_exile_step()`: keep the delegation to the shared helper. Verified behavior-preserving: the shared `leave_battlefield_exile_replacement_impl()` is field-for-field identical to the inline form main built (`ReplacementEvent::Moved` + `valid_card: SelfRef` + `expiry: UntilHostLeavesPlay` + `ChangeZone` Battlefield->Exile on `SelfRef`). Note the auto-merge of parser/oracle_effect/mod.rs correctly grafted main's newly added `.expiry(RestrictionExpiry::UntilHostLeavesPlay)` stamp into the extracted helper, which the PR head lacked -- main's fix is preserved, not reverted.
…ust actually fire A quoted grant of a replacement-shaped ability — Geth, Thane of Contracts' 'It gains "If this creature would leave the battlefield, exile it instead of putting it anywhere else"' — parsed to an inert ContinuousModification::GrantAbility. The concretized body was pushed into obj.abilities and never into replacement_definitions, so the redirect silently did nothing and the reanimated creature went to the graveyard on any battlefield exit. Adds ContinuousModification::GrantReplacement, the replacement-store sibling of GrantAbility/GrantTrigger/GrantStaticAbility (CR 614 is a categorically distinct sink from CR 602/603/604). It is applied at layer 6 with structural dedup and re-derived every layer pass (CR 613.1f), so its lifetime is governed by the granting continuous effect rather than an object-lifetime stamp: a permanent grant survives its source leaving, and a temporary one (Elemental Expressionist's "Until end of turn") lapses at cleanup (CR 611.2a, CR 514.2). A grant carrying other modifications alongside the replacement is never promoted to permanent — Expressionist grants two abilities, so that guard is load-bearing and is pinned by its own regression. Also fixes the parser gap that made the rider unreachable at this seam: normalize_card_name_refs rewrites "this creature|permanent|land" to "~" card-wide (CR 201.5b), but parse_leave_battlefield_rider_ref had no "~" arm, so every quoted grant failed to parse. The standalone rider path (phase-rs#6538) is unaffected — no standalone printing uses a "this <type>" subject. The Moved→Exile ReplacementDefinition is built by one shared, deliberately UNSTAMPED constructor (leave_battlefield_exile_replacement, expiry: None) with three consumers that decide the lifetime themselves: * the standalone parser rider (try_parse_leave_battlefield_exile_ replacement) and unearth's synthesis (CR 702.84a) each compose .expiry(UntilHostLeavesPlay) — reproducing phase-rs#6538's host-lifetime stamp byte-for-byte (CR 400.7), so phase-rs#6538's base-install / non-copiable / host-exit-prune machinery is preserved exactly; and * the granted path (classify_quoted_inner → GrantReplacement) uses the parser fn purely as a DETECTOR and grants the UNSTAMPED constructor output. A granted replacement's lifetime is governed by the grant's duration and re-derived every layer pass (CR 611.2a / CR 613.1f); a host-lifetime stamp would be read by phase-rs#6538's is_runtime_host_lifetime_replacement (base-install + non-copiable + host-exit-prune), base-installing the granted rider so it would outlive the granting continuous effect and Expressionist's until-end-of-turn grant would never lapse. The reconciliation is pinned by granted_replacement_is_not_host_lifetime_stamped (RED if the granted def is ever re-stamped) and the byte-identity test leave_battlefield_exile_replacement_matches_6538_shape (RED if the standalone shape drifts from phase-rs#6538). Closes phase-rs#6566 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ust actually fire (phase-rs#6580) A quoted grant of a replacement-shaped ability — Geth, Thane of Contracts' 'It gains "If this creature would leave the battlefield, exile it instead of putting it anywhere else"' — parsed to an inert ContinuousModification::GrantAbility. The concretized body was pushed into obj.abilities and never into replacement_definitions, so the redirect silently did nothing and the reanimated creature went to the graveyard on any battlefield exit. Adds ContinuousModification::GrantReplacement, the replacement-store sibling of GrantAbility/GrantTrigger/GrantStaticAbility (CR 614 is a categorically distinct sink from CR 602/603/604). It is applied at layer 6 with structural dedup and re-derived every layer pass (CR 613.1f), so its lifetime is governed by the granting continuous effect rather than an object-lifetime stamp: a permanent grant survives its source leaving, and a temporary one (Elemental Expressionist's "Until end of turn") lapses at cleanup (CR 611.2a, CR 514.2). A grant carrying other modifications alongside the replacement is never promoted to permanent — Expressionist grants two abilities, so that guard is load-bearing and is pinned by its own regression. Also fixes the parser gap that made the rider unreachable at this seam: normalize_card_name_refs rewrites "this creature|permanent|land" to "~" card-wide (CR 201.5b), but parse_leave_battlefield_rider_ref had no "~" arm, so every quoted grant failed to parse. The standalone rider path (phase-rs#6538) is unaffected — no standalone printing uses a "this <type>" subject. The Moved→Exile ReplacementDefinition is built by one shared, deliberately UNSTAMPED constructor (leave_battlefield_exile_replacement, expiry: None) with three consumers that decide the lifetime themselves: * the standalone parser rider (try_parse_leave_battlefield_exile_ replacement) and unearth's synthesis (CR 702.84a) each compose .expiry(UntilHostLeavesPlay) — reproducing phase-rs#6538's host-lifetime stamp byte-for-byte (CR 400.7), so phase-rs#6538's base-install / non-copiable / host-exit-prune machinery is preserved exactly; and * the granted path (classify_quoted_inner → GrantReplacement) uses the parser fn purely as a DETECTOR and grants the UNSTAMPED constructor output. A granted replacement's lifetime is governed by the grant's duration and re-derived every layer pass (CR 611.2a / CR 613.1f); a host-lifetime stamp would be read by phase-rs#6538's is_runtime_host_lifetime_replacement (base-install + non-copiable + host-exit-prune), base-installing the granted rider so it would outlive the granting continuous effect and Expressionist's until-end-of-turn grant would never lapse. The reconciliation is pinned by granted_replacement_is_not_host_lifetime_stamped (RED if the granted def is ever re-stamped) and the byte-identity test leave_battlefield_exile_replacement_matches_6538_shape (RED if the standalone shape drifts from phase-rs#6538). Closes phase-rs#6566 Co-authored-by: michiot05 <281539540+michiot05@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ve layer resets (Rotting Rats + Sacrifice) (phase-rs#6538) Unearth installs a rider — "if it would leave the battlefield, exile it instead of putting it anywhere else" (CR 702.84a) — on the returned creature, but it was pushed to the object's LIVE replacement set only. Every layer pass reseeds live characteristics from base (CR 613.1), so the rider was wiped before any real leave event; sacrificing, destroying, or bouncing an unearthed creature sent it to the graveyard instead of exile. Only the end-step delayed-trigger half worked (it is state-hosted, not object-hosted). Adds a typed RestrictionExpiry::UntilHostLeavesPlay stamped on the rider at both build sites (unearth synthesis + the Whip-of-Erebos-class parser builder). The stamp (1) forces base-install so the rider survives CR 613.1 reseeds, (2) marks the def non-copiable (CR 707.2 — token copies never inherit it), and (3) prunes it base+live when the host leaves the battlefield (CR 400.7 — a re-entered same-ObjectId object is a new object and must not be re-exiled). Object-bound, so it survives control theft. Fixes ~58 unearth cards + 6 leave-battlefield-rider cards (Whip of Erebos, Gruesome Encore, Isareth, Kheru Lich Lord, Moira and Teshar, From the Catacombs). Personal Decoy's printed static (no expiry stamp) stays copiable — the detectors key on the stamp, not the shape. Closes phase-rs#5976 Co-authored-by: michiot05 <281539540+michiot05@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Tier: Frontier
Model: claude-opus-4-8
Closes #5976.
Summary
Rotting Rats returned via Unearth and then sacrificed to Sacrifice went to the graveyard instead of exile. Unearth's rider — "Exile it at the beginning of the next end step or if it would leave the battlefield" (CR 702.84a) — has two halves, and only the end-step delayed trigger worked; the "if it would leave the battlefield, exile it instead" replacement did nothing on a sacrifice, destroy, or bounce.
Root cause is not the sacrifice path (that correctly routes through
sacrifice_permanent→replace_event, and the matcher accepts the rider). The rider was installed onto the returned creature's livereplacement_definitionsonly.seed_live_characteristics_from_basereseeds live characteristics from base on every layer pass (CR 613.1) — and unearth's return grants haste, forcing an immediate layer evaluation — so the rider was wiped before any real leave event ever reached it. The end-step half survived only because it is a state-hosted delayed trigger, not an object-hosted replacement.Class-level fix, not a Rotting-Rats special case: a typed
RestrictionExpiry::UntilHostLeavesPlay, stamped on the rider at both build sites (the unearth synthesizer and the sharedtry_parse_leave_battlefield_exile_replacementparser builder that also handles the Whip-of-Erebos rider class). The stamp threads through the three existing runtime-rider seams that the turn-bound die-exile rider (Torch the Tower) already established:add_target_replacement.rs) — the rider persists inbase_replacement_definitions, so it survives every CR 613.1 reseed.printed_cards.rs, CR 707.2) — token/merge/augment copies never inherit it (mana isn't copied; a copy isn't the cast object).layers.rs, CR 400.7) — on the host's battlefield exit (including the exile redirect itself) the rider is dropped from base+live, so a re-entered same-ObjectIdobject is a new object and is not wrongly re-exiled.Because the rider is bound to the object (
valid_card: SelfRef), it survives control theft — a stolen unearthed creature sacrificed by its new controller is still exiled.Implementation method (required)
Method: /engine-implementer — plan → /review-engine-plan (1 round; findings integrated: corrected class inventory, verbatim-card-text discipline, prune reach-guard, variant-doc scoping) → implement → verification pass (one test-harness foot-gun fixed, no production defect) → /review-impl (CLEAN, zero findings) → commit → final read-only /review-impl: PASS head=a65dd1a2229f2c231fab50579e346df2901e2239. Each step in a fresh agent context.
Gate A
./scripts/check-parser-combinators.sh→ Gate A PASS head=a65dd1a2229f2c231fab50579e346df2901e2239 (Gate G PASS). The parser change is a single.expiry(...)stamp on an already-builtReplacementDefinition— no string probes, no nom-chain change.Anchored on
crates/engine/src/database/unearth.rs(leaves_battlefield_exile_step) — the synthesized unearth rider, now stamped; andcrates/engine/src/game/layers.rs:1692(seed_live_characteristics_from_base) — the CR 613.1 reseed that wiped the live-only rider (the root cause).EndOfTurnexpiry) — the exact three-seam durable-runtime-rider pattern this change mirrors:is_runtime_target_die_exile_replacement(printed_cards.rs) +install_to_base(add_target_replacement.rs) + the host-left prune (layers.rs::prune_controller_controls_source_on_leave). The new predicateis_runtime_host_lifetime_replacementis the object-lifetime sibling.crates/engine/src/types/ability.rs—Duration::UntilHostLeavesPlay(the existing object-lifetime vocabulary) established that host-bound and turn-bound lifetimes coexist as distinct categorical members; the newRestrictionExpiryvariant is the replacement-expiry counterpart.Verification
cargo fmt --allclean;cargo clippy -p engine --tests0 warnings.crates/engine/tests/integration/issue_5976_unearth_leave_redirect.rs, registered inmain.rs) — 8 rows, each driving the real pipeline (unearth activate/resolve → a genuine layer pass → real leave event → zone assert, withPermanentSacrificed/damage/priority reach guards): the reported additional-cost sacrifice (verbatim Sacrifice card) → exile; ability-cost sacrifice outlet → exile; destroy → exile; bounce → exile (any leave event, CR 702.84a); non-unearthed sibling → graveyard; host-exit prune with no revival on re-entry; control-theft object-binding; and the real parser-rider card Gruesome Encore (verbatim) → bounce → exile.printed_cardspredicate tests (non-copiable exclusion; and the Personal Decoy printed-static shape — no expiry stamp — staying copiable, the negative that proves the detector keys on the stamp, not the redirect shape); parser expiry-stamp assertion. All green.cargo test -p engine --lib -- replacement layer copiable torch_the_tower ninjutsu die_exile→ 2398 passed, 0 failed (Torch-the-TowerEndOfTurndie-exile, copy/merge/augment, layers, ninjutsu all intact).Graveyard/Hand(left: Graveyard, right: Exile) while the non-unearthed sibling R5 stays green; restored → all pass.docs/MagicCompRules.txt— CR 702.84a, 400.7, 613.1, 614.1a, 707.2, 611.2b.Claimed parse impact
Regenerated
card-data.jsonand isolated the change from the pre-existingDuration::UntilHostLeavesPlaynamesake (288 pre-existingdurationoccurrences on the unrelated "exile until ~ leaves" family, untouched): exactly 64 cards gain an additiveexpiry: UntilHostLeavesPlayfield on their existing leave-battlefield exile replacement — the complete class (~58 unearth cards: Rotting Rats, Hellspark Elemental, Fatestitcher, the Scrapwork/Dregscape cycle, Sedraxis Specter, … + the 6–7 parser-rider cards: Whip of Erebos, Gruesome Encore, From the Catacombs, Isareth the Awakener, Kheru Lich Lord, Moira and Teshar, Dreams of the Dead). No card gains or loses support — the replacement already existed; it now carries the host-lifetime stamp and is made durable. The CI parse-diff artifact should report these 64 signature changes.Scope Expansion
None.
🤖 Generated with Claude Code
Summary by CodeRabbit