fix(parser): recognize plural "leave the battlefield" in the generic exile-until duration combinator#5871
Conversation
Cloak and Dagger, Entwined's ETB exiles a nonland card from the chosen opponent's hand "until Cloak and Dagger leave the battlefield" — plural verb agreement, since the card's own name is a plural subject. The "until X leaves the battlefield" duration combinator (parser/oracle_nom/duration.rs) only matched the singular "leaves" form, so `duration` silently stayed None and the exiled card never came back when the source left the battlefield (no ExileLinkKind::UntilSourceLeaves link was ever created). The sibling *trigger* detector for LTB already accepted both "leaves"/"leave" (oracle_trigger.rs, oracle_effect/mod.rs); the duration combinator just never got the same alternative. Fix: accept "leave the battlefield" alongside "leaves the battlefield" in parse_until_body, mirroring the existing trigger-side alternation. Added a regression test (issue_4235_cloak_and_dagger_entwined.rs) covering both the AST-level duration fix and a full runtime cast -> reveal -> exile -> destroy source -> return-to-hand pipeline. Two separate, unrelated gaps were found during investigation but are NOT fixed here (documented in the test's module doc and in investigated-issues.md): (1) the "or the chosen creature" alternative exile target has no parser support at all, and neither does the "up to one target creature they control" secondary target it depends on; (2) the exile filter isn't scoped to the specific chosen opponent's hand (matches a nonland card in either player's hand), and (3) WaitingFor::EffectZoneChoice's interactive round-trip drops `duration` entirely regardless of card. Both are real but architecturally larger than this scoped, card-specific bugfix warrants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the duration parser in duration.rs to accept both singular ('leaves') and plural ('leave') verb forms for host-lifetime expiry, fixing a bug where cards with plural names (such as 'Cloak and Dagger, Entwined') would not correctly apply their 'until leaves the battlefield' duration. A comprehensive integration test was added to verify this behavior. The review feedback points out that the code comment cites an incorrect MTG Comprehensive Rule (CR) and violates the style guide's mandatory CR annotation format, suggesting a correction to use CR 611.2b with the proper description.
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.
| // "leaves the battlefield"/"leave the battlefield" alt) — CR 603.2c | ||
| // notes the plural "leave" form is used with plural/batched subjects. |
There was a problem hiding this comment.
[MEDIUM] Incorrect CR citation and format.
Why it matters: The comment cites CR 603.2c (which governs triggered abilities triggering once per event) for a continuous effect duration parser, and does not follow the required CR <number>: <description> format. It should cite CR 611.2b which specifically governs continuous effects with 'until [something] leaves the battlefield' durations.
Suggested fix: Update the comment to use the correct CR 611.2b citation and format.
| // "leaves the battlefield"/"leave the battlefield" alt) — CR 603.2c | |
| // notes the plural "leave" form is used with plural/batched subjects. | |
| // "leaves the battlefield"/"leave the battlefield" alt). | |
| // CR 611.2b: Some continuous effects generated by the resolution of a spell or ability have durations phrased "until [something] leaves the battlefield." |
References
- Every rules-touching line of engine code must carry a comment of the form
CR <number>: <description>. The cited rule's body must describe what the code is doing. (link)
Parse changes introduced by this PR · 1 card(s), 1 signature(s) (baseline: main
|
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Verdict: Changes requested
🔴 Blocker
-
[HIGH] The newly parsed duration is lost on the ordinary multi-candidate path.
crates/engine/src/game/engine_resolution_choices.rsreconstructsChangeZoneIterationCtxfromWaitingFor::EffectZoneChoicewithduration: None, as this PR's own module documentation notes. The runtime fixture deliberately removes every competing eligible card so it takes the single-candidate shortcut; it therefore cannot prove the advertised card works when an opponent has two nonland cards. Thread the duration through the choice/resume authority and add a two-eligible-card runtime test that selects one, then verifies the exile link returns it when Cloak and Dagger leaves. -
[HIGH] This leaves coverage dishonest for the full Oracle sentence. The parser still accepts Cloak and Dagger while dropping the
or the chosen creaturealternative, the secondaryup to one target creaturedeclaration it depends on, and the chosen-opponent hand binding. The test's direct zone mutation works around that last error rather than exercising the actual card. Preserve the clause as a strict unsupported marker until its target/anaphor binding is implemented, or complete the general engine/parser work in the same PR.
🟡 Non-blocking
- [MED] The new
CR 603.2ccitation is unrelated. That rule is about repeated triggering from one event; the verified duration rule isCR 611.2a. Correct the annotation while reworking the duration path.
✅ Clean
- The plural
leaveparser alternative is composed at the existing duration combinator and the parse-diff is limited to Cloak and Dagger.
Recommendation: Complete the general choice-resume and Oracle-clause handling, then resubmit with a multi-candidate end-to-end regression.
2 participants
Fixes #4235.
Cloak and Dagger, Entwined: "...You may exile a nonland card from their hand or the chosen creature until Cloak and Dagger leave the battlefield."
Root cause & fix
parse_until_body(the generic "until [source] leaves the battlefield" duration combinator) only matched the singular verb form "leaves the battlefield". Cloak and Dagger, Entwined's own name is a plural subject, so its printed oracle text uses plural agreement — "until Cloak and Dagger leave the battlefield" — which never matched. This left the exile sub-ability'sdurationfield silentlyNone, so noExileLinkKind::UntilSourceLeaveslink was ever created, and a card exiled by the ETB trigger never returned when Cloak and Dagger left the battlefield.The sibling trigger detector for leaves-the-battlefield already handled both "leaves"/"leave" forms — only the duration combinator was missing the alternative. Fixed by mirroring that existing alternation.
Testing
New regression test (AST-level + full runtime cast→reveal→exile→destroy-source→return-to-hand) — confirmed RED without the fix, GREEN with it.
Verified locally (rebased onto main): fmt clean, clippy clean, 16672 lib tests pass, 3129 integration tests pass.