Skip to content

test(engine): regression for CR 704.5m aura cleanup when host is exiled#6054

Merged
matthewevans merged 1 commit into
phase-rs:mainfrom
andriypolanski:fix/5999-aura-graveyard-on-host-exile
Jul 16, 2026
Merged

test(engine): regression for CR 704.5m aura cleanup when host is exiled#6054
matthewevans merged 1 commit into
phase-rs:mainfrom
andriypolanski:fix/5999-aura-graveyard-on-host-exile

Conversation

@andriypolanski

Copy link
Copy Markdown
Contributor

Fixes #5999

Summary

Discord report (#5999): enchantment Auras stay on the battlefield when the permanent they are attached to is exiled.

Investigation showed the engine already handles this correctly on main:

  • CR 704.5msba::check_unattached_auras puts an Aura into its owner's graveyard when is_valid_attachment_target fails (host not on the battlefield).
  • CR 704.4 — SBAs are intentionally deferred while a spell is still resolving. Spells with an optional rider after the exile clause (e.g. Path to Exile's land search) pause on OptionalEffectChoice with the stack already empty; the Aura correctly remains on the battlefield until that rider is answered.
  • Once the optional rider resolves, apply_actionrun_post_action_pipeline runs the SBA loop and the Aura goes to the graveyard.

The gap was test coverage and a misleading initial repro driver: advance_until_stack_empty() only passes priority and never answers OptionalEffectChoice, so the test stopped with the Aura still on the battlefield even though the engine was waiting for a legitimate player decision.

Changes

  • crates/engine/tests/integration/issue_5999_aura_exile_host.rs — four regression tests:
    • Direct exile + SBAmove_to_zone(host, Exile) then check_state_based_actions; Aura graves immediately.
    • Simple exile spell — unconditional Exile target creature. through the full cast/resolve pipeline.
    • Path to Exile — full pipeline with optional land-search rider declined; Aura graves after resolution settles.
    • CR 704.4 pause — asserts Aura stays on the battlefield while the optional rider is pending, then graves once declined.
  • crates/engine/tests/integration/main.rs — register the new module.

No engine, parser, or frontend code changes — behavior was already correct.

Test plan

cargo fmt --all -- --check
cargo test -p engine issue_5999

Expected: all four integration tests pass.

cargo test -p engine issue_5999_aura_goes_to_graveyard_when_host_is_exiled_directly
cargo test -p engine issue_5999_aura_goes_to_graveyard_when_host_is_exiled_by_simple_exile_spell
cargo test -p engine issue_5999_aura_goes_to_graveyard_when_enchanted_creature_is_exiled
cargo test -p engine issue_5999_aura_stays_on_battlefield_until_optional_rider_resolves

Related aura SBA regressions (should remain green):

cargo test -p engine issue_2863

Notes for reviewers

  • Path to Exile exiles the creature before prompting its controller for the optional land search. During that prompt the Aura is still on the battlefield — that is rules-correct (CR 704.4), not a missing SBA.
  • If players report the Aura "stuck" after exile, verify the UI surfaces OptionalEffectChoice when the stack is empty but resolution is not finished (the exiled creature's controller must answer the land-search prompt).
  • Spells without an optional post-exile rider (e.g. plain Exile target creature.) graveyard the Aura as soon as the spell fully resolves, with no intermediate pause.

@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 16, 2026
@superagent-security

Copy link
Copy Markdown

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

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@matthewevans matthewevans self-assigned this Jul 16, 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.

Reviewed — test-only regression coverage is substantively distinct; hold for current required Rust check.

✅ Clean

  • issue_5999_aura_exile_host.rs drives the production cast/stack/optional-choice path and proves the CR 704.4 boundary: the Aura remains while Path to Exile is paused, then CR 704.5m removes it after the decision settles. The saw_optional reach guard makes that timing assertion non-vacuous.
  • The direct SBA case complements, rather than replaces, existing target-leaves coverage: it pins the Zone::Exile host path, while the two full-pipeline cases cover unconditional and optional-rider resolution.
  • Existing sba::check_unattached_auras is the correct ownership seam; this PR adds no production behavior or duplicate mechanism. No parser-surface change means no parse-diff artifact is required.

Recommendation: hold this exact head until the required Rust aggregate finishes, then return it for the authorized handler approval/enqueue pass.

@matthewevans matthewevans added the test Add tests label Jul 16, 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 complete: the registered production-pipeline tests cover the previously misleading optional-choice boundary and the direct SBA seam; required checks are green.

@matthewevans
matthewevans added this pull request to the merge queue Jul 16, 2026
@matthewevans matthewevans removed their assignment Jul 16, 2026
Merged via the queue into phase-rs:main with commit cea140e Jul 16, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. test Add tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enchantment Auras not going to graveyard — Enchantment Auras are not going to graveyard if the permanent they were atta…

3 participants