fix(engine): keep prepared-copy spells stack-resident through targeting

[nishu-builder]

Summary

Keeps a prepared-copy spell alive while its cast is paused for targeting. CastPreparedCopy created the copy object in Exile with is_token = true and then paused for target selection; a state-based-action pass during the pause removed the token (outside battlefield/stack), and completing ChooseTarget emitted SpellCast against a destroyed object — leaving a stack entry referencing an absent object. Found by an external deterministic-replay harness (11 distinct seeds; repro: a prepared back-face Regrowth copy targeting a graveyard card), with both actions drawn from the engine's own exact legal-action set. The fix aligns the prepared-copy path with ordinary announcement: the copy is stack-resident (same-id StackEntryKind::Spell) from cast start, so CR 704.5d/e token cleanup correctly ignores it, and it ceases to exist through the normal post-resolution route. The residency check requires the same-id entry to be StackEntryKind::Spell specifically — same-id non-spell projections (virtual activated-ability overlays) neither shield an off-battlefield token from SBAs nor bypass the movement guards, with a hostile fixture proving both.

Files changed

• crates/engine/src/game/sba.rs
• crates/engine/src/game/zone_pipeline.rs
• crates/engine/src/game/zones.rs
• crates/engine/tests/integration/issue_1312_prepared_spell_cast_triggers.rs

CR references

• CR 109.1
• CR 601.2a
• CR 602.2a
• CR 603.3
• CR 704.5d
• CR 704.5e

Implementation method (required)

Method: /engine-implementer

Track

Developer

LLM

Model: gpt-5.6-sol
Thinking: high
Tier: Frontier

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.

• tilt get uiresource clippy — Tilt unavailable in this worktree; used the documented direct fallback.

• cargo fmt --all / cargo fmt --all -- --check — passed.

• git diff --check — passed.

• cargo clippy -p phase-engine --all-targets -- -D warnings — passed on head f569c3753289f763d95fc7ea5644bdf8ff7b6a5b.

• cargo test -p phase-engine — passed on head f569c3753289f763d95fc7ea5644bdf8ff7b6a5b (full suites, 0 failed).

• Plan verification matrix — passed individually: SBA-pass survival regression (production CastPreparedCopy → explicit sba::check_state_based_actions pass mid-targeting → ChooseTarget completes → stack object exists → normal resolution), countered-copy sibling (copy ceases correctly), hostile same-id non-spell fixture (virtual ActivatedAbility overlay does not shield SBA removal nor pass movement guards), raw-movement and zone-pipeline hostile fixtures, three direct-to-stack sibling tests.

• Revert probe — with the raw-zone-only residency check restored, the survival regression fails; with the fix, passes.

• ./scripts/gen-card-data.sh — passed on head f569c3753289f763d95fc7ea5644bdf8ff7b6a5b.

• cargo coverage — passed on head f569c3753289f763d95fc7ea5644bdf8ff7b6a5b: timeless legal 15124/16180 fully supported (93.5%); vintage legal 29841/32268 fully supported (92.5%).

• cargo semantic-audit — passed on head f569c3753289f763d95fc7ea5644bdf8ff7b6a5b: 32732 cards audited, 297 existing findings.

Gate A

Gate A PASS head=f569c3753289f763d95fc7ea5644bdf8ff7b6a5b base=6d7821dced9623609edea342b47dd9c704ff0b36

Anchored on

• crates/engine/src/game/casting.rs:12086 — announce_spell_on_stack binds the spell object's exact id to StackEntryKind::Spell at announcement; the prepared-copy path now establishes the same residency at the same point in its cast.
• crates/engine/src/game/effects/cast_from_zone.rs:933 — cast_single_target_during_resolution's direct-to-stack copy casting is the analogous copy-cast seam whose object lifetime the fix mirrors.

Final review-impl

Final review-impl PASS head=f569c3753289f763d95fc7ea5644bdf8ff7b6a5b

Claimed parse impact

None.

Validation Failures

Contributor-environment note per the engine-implementer skill: pipeline steps ran as isolated fresh contexts (Codex CLI sessions) with artifact-only handoffs rather than spawned Claude subagents. Plan review: 3 rounds to clean (1 → 1 → 0; round one tightened the residency contract to StackEntryKind::Spell against same-id virtual ability overlays, round two corrected the rules rationale to CR 109.1/602.2a/603.3 after an invalid CR 111.1b citation was caught). Implementation review: 1 round, clean.

CI Failures

None.

Related

Same contributor as #6997 (merged), #7007, #7008; independent concerns.

Summary by CodeRabbit

• Bug Fixes
  • Corrected lifecycle handling for tokens and card copies that leave the battlefield.
  • Prevented tokens from being incorrectly treated as spells when sharing an ID with ability entries on the stack.
  • Ensured prepared spell copies remain available through targeting and are removed correctly after resolution or countering.
  • Added regression coverage for stack cleanup, exile behavior, and synthetic spell handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 71cd9cad-95f2-4b6b-ab84-a6800bbd34bc

📥 Commits

Reviewing files that changed from the base of the PR and between 4756407 and 7c8fb95.

📒 Files selected for processing (1)

• crates/engine/src/game/sba.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/engine/src/game/sba.rs

---

📝 Walkthrough

Walkthrough

The change makes token and card-copy lifecycle checks state-aware. Only live pending spell entries preserve stack residency. Regression tests cover cleanup, zone movement, prepared-copy resolution, and countering.

Changes

Token and copy residency

Layer / File(s)	Summary
State-aware stack residency crates/engine/src/game/zones.rs	object_has_stack_residency now requires stack presence or a matching live pending Spell entry. Ability entries no longer qualify.
Cleanup and zone movement integration crates/engine/src/game/sba.rs, crates/engine/src/game/zone_pipeline.rs, crates/engine/src/game/zones.rs	State-based cleanup and token movement guards now use GameState. Tests cover bare spell placeholders and activated-ability entries.
Prepared spell lifecycle coverage crates/engine/tests/integration/issue_1312_prepared_spell_cast_triggers.rs	Prepared-copy tests now validate stack residency, target finalization, normal resolution, countering, and synthetic-copy removal.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: keeping prepared-copy spells stack-resident during targeting.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Generated for head 7c8fb958d757e9891549ba2627a43c0cf174d662.

Parse changes introduced by this PR

✓ No card-parse changes detected.

[matthewevans]

[MED] Scope prepared-copy stack residency to the production casting/SBA boundary rather than globally exempting same-ID placeholders. Evidence: crates/engine/src/game/zones.rs:25 now treats any matching StackItem::Spell as residency, while the new regression test manually calls check_state_based_actions during TargetSelection at crates/engine/tests/integration/issue_1312_prepared_spell_cast_triggers.rs:105; normal cast flow reaches state-based actions at the priority boundary (crates/engine/src/game/engine.rs:11051, crates/engine/src/game/engine_priority.rs:180). Why it matters: this global predicate lets an Exile token/copy survive whenever an unrelated same-ID spell placeholder exists, even outside the actual cast lifecycle; the test manufactures that condition before target finalization turns the object into Zone::Stack.

Please remove or rework the global exemption and add a production-path reproduction that advances through the real legal-action/cast flow to the normal SBA boundary. If debug/replay handling needs a special policy, keep it at that explicit API rather than changing the shared zone-residency rule. The revised test should demonstrate both that the prepared copy survives the real targeting path and that the synthetic non-stack case is still cleaned up.

[nishu-builder]

Both points addressed in 798fefe:

1. Lifecycle-scoped residency — the zones.rs predicate no longer treats bare same-ID StackItem::Spell existence as residency. Protection is bound to the exact pending-cast lifecycle (announcement placeholder through stack commitment); a same-ID placeholder outside a live cast neither shields an exile token from CR 704.5d/e cleanup nor passes the movement guards, proven by the retained synthetic non-lifecycle fixture.

2. Production-path reproduction — the regression no longer invokes check_state_based_actions manually during TargetSelection. It drives the real legal-action cast flow (CastPreparedCopy → targeting pause → natural SBA execution at the priority boundary per engine.rs:11051 / engine_priority.rs:180 → ChooseTarget completes → normal resolution). The countered-copy sibling stays green.

Verification on head 798fefe7646139626b52715f0f06527ad242f654: fmt --check, clippy -D warnings, issue_1312 module, full cargo test -p phase-engine (0 failures), revert probe (production-path regression fails with the lifecycle binding reverted, passes restored). Gate A PASS head=798fefe7646139626b52715f0f06527ad242f654 base=6d7821dced9623609edea342b47dd9c704ff0b36. Final review-impl PASS head=798fefe7646139626b52715f0f06527ad242f654 (fresh-context).

Model: gpt-5.6-sol

[matthewevans]

I re-reviewed the current head 4756407f0f3d66ed706ab310a082716468501d0b. The implementation has no remaining substantive review finding; one low-severity test suggestion is non-blocking.

Holding approval briefly for the current-head CI evidence: Rust lint, Rust tests, and Card data are still running. The existing parse-diff receipt is for stale head f569c3753289f763d95fc7ea5644bdf8ff7b6a5b, so it cannot support this head. Once those checks and a parse-diff receipt bound to 4756407f0f3d66ed706ab310a082716468501d0b are available, I will re-check and proceed with approval if they remain clean.

[matthewevans]

Maintainer hold for current head 7c8fb95.

The branch was brought current with main and the implementation review remains clean, but this new head is awaiting its required CI plus a parse-diff receipt bound to this exact SHA. After those are green/current, re-check the live head and proceed with fresh approval and merge-queue enqueue.

[matthewevans]

Maintainer review is clean at current head 7c8fb95; required checks and the exact-head parse-diff receipt are green. Approving for merge queue.