fix(parser): parse "Activate only during an opponent's upkeep" (Trade…

crates/engine/src/ai_support/filter.rs

@@ -918,6 +918,8 @@ fn condition_reads_only_memo_safe_state(c: &ParsedCondition) -> bool {
         | ParsedCondition::CardsLeftYourGraveyardThisTurnAtLeast { .. }
         | ParsedCondition::PlayerCountAtLeast { .. }
         | ParsedCondition::HasCityBlessing
+        // CR 503.1: reads only `state.phase`, apply()-constant global state.
+        | ParsedCondition::IsDuringUpkeep
         | ParsedCondition::IsYourTurn => true,
     }
 }

crates/engine/src/game/restrictions.rs

@@ -1518,6 +1518,9 @@ pub(crate) fn evaluate_condition(
         ParsedCondition::HasCityBlessing => state.city_blessing.contains(&player),
         // CR 102.1: "The active player is the player whose turn it is."
         ParsedCondition::IsYourTurn => state.active_player == player,
+        // CR 503.1: The game is currently in the upkeep step. Player scope, if
+        // any, is composed by the caller via `And([Not(IsYourTurn), ..])`.
+        ParsedCondition::IsDuringUpkeep => state.phase == Phase::Upkeep,
         // CR 601.3d + CR 608.2c: "if it targets a [filter]" — gates a casting
         // permission on the chosen targets of the in-flight spell. Read from
         // `state.pending_cast.ability.targets` when targets have been committed.
@@ -3092,6 +3095,54 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn opponents_upkeep_activation_condition_gates_on_step_and_scope() {
+        // CR 602.5b + CR 102.1 + CR 503.1: "Activate only during an opponent's
+        // upkeep" is the composed restriction the parser emits for Trade Caravan —
+        // `And([Not(IsYourTurn), IsDuringUpkeep])`. Drive the real
+        // `evaluate_condition` across the full turn-scope × step matrix so the gate
+        // holds only in an opponent's upkeep step.
+        let mut state = crate::types::game_state::GameState::new_two_player(42);
+        let condition = ParsedCondition::And {
+            conditions: vec![
+                ParsedCondition::Not {
+                    condition: Box::new(ParsedCondition::IsYourTurn),
+                },
+                ParsedCondition::IsDuringUpkeep,
+            ],
+        };
+        let activator = PlayerId(0);
+
+        // Opponent's turn, upkeep step -> allowed.
+        state.active_player = PlayerId(1);
+        state.phase = Phase::Upkeep;
+        assert!(evaluate_condition(
+            &state,
+            activator,
+            ObjectId(1),
+            &condition
+        ));
+
+        // Opponent's turn, non-upkeep step -> denied (IsDuringUpkeep false).
+        state.phase = Phase::PreCombatMain;
+        assert!(!evaluate_condition(
+            &state,
+            activator,
+            ObjectId(1),
+            &condition
+        ));
+
+        // Your turn, upkeep step -> denied (Not(IsYourTurn) false).
+        state.active_player = PlayerId(0);
+        state.phase = Phase::Upkeep;
+        assert!(!evaluate_condition(
+            &state,
+            activator,
+            ObjectId(1),
+            &condition
+        ));
+    }
+
     #[test]
     fn evaluates_creatures_you_control_total_power_condition() {
         let mut state = crate::types::game_state::GameState::new_two_player(42);

crates/engine/src/parser/oracle.rs

@@ -6781,6 +6781,13 @@ fn parse_activation_during_role_gate(i: &str) -> OracleResult<'_, ActivationRest
             tag::<_, _, OracleError<'_>>("as a sorcery"),
         ),
         value(ActivationRestriction::AsInstant, tag("as an instant")),
+        value(
+            opponents_upkeep_activation_restriction(),
+            alt((
+                tag("during an opponent's upkeep"),
+                tag("during an opponents upkeep"),
+            )),
+        ),
         value(
             opponents_turn_activation_restriction(),
             alt((
@@ -7010,6 +7017,24 @@ fn opponents_turn_activation_condition() -> ParsedCondition {
     }
 }
 
+/// CR 602.5b + CR 102.1 + CR 503.1: "Activate only during an opponent's upkeep"
+/// gates activation to the upkeep step of a turn where the activator is not the
+/// active player (Trade Caravan). Composed from the same `Not(IsYourTurn)`
+/// opponent-turn leaf as `opponents_turn_activation_condition` plus the
+/// `IsDuringUpkeep` step predicate, so the opponent scope reuses the existing
+/// composition idiom instead of a dedicated `DuringOpponents*` restriction
+/// sibling.
+fn opponents_upkeep_activation_restriction() -> ActivationRestriction {
+    ActivationRestriction::RequiresCondition {
+        condition: Some(ParsedCondition::And {
+            conditions: vec![
+                opponents_turn_activation_condition(),
+                ParsedCondition::IsDuringUpkeep,
+            ],
+        }),
+    }
+}
+
 pub(super) fn strip_activated_constraints(text: &str) -> (String, ActivatedConstraintAst) {
     let mut remaining = text.trim().trim_end_matches('.').trim().to_string();
     let mut constraints = ActivatedConstraintAst::default();

crates/engine/src/parser/oracle_tests.rs

@@ -248,6 +248,53 @@ fn activated_ability_opponent_turn_restriction_uses_not_your_turn_condition() {
     );
 }
 
+/// CR 602.5b + CR 102.1 + CR 503.1: "Activate only during an opponent's upkeep"
+/// (Trade Caravan) composes the opponent-turn scope (`Not(IsYourTurn)`) with the
+/// upkeep-step predicate (`IsDuringUpkeep`) inside a single `RequiresCondition`,
+/// reusing the same composition idiom as the bare opponent-turn gate above rather
+/// than a dedicated `DuringOpponents*` restriction sibling. Before the fix the
+/// tail dropped to `Effect::Unimplemented` and the ability was activatable at any
+/// time.
+#[test]
+fn activated_ability_opponents_upkeep_restriction_composes_scope_and_step() {
+    let r = parse(
+        "Remove two currency counters from ~: Untap target basic land. \
+         Activate only during an opponent's upkeep.",
+        "Trade Caravan",
+        &[],
+        &["Creature"],
+        &["Human", "Nomad"],
+    );
+
+    assert_eq!(r.abilities.len(), 1, "got {:#?}", r.abilities);
+    let ability = &r.abilities[0];
+    assert!(
+        !matches!(ability.effect.as_ref(), Effect::Unimplemented { .. })
+            && ability
+                .sub_ability
+                .as_ref()
+                .is_none_or(|sub| !matches!(sub.effect.as_ref(), Effect::Unimplemented { .. })),
+        "expected no unimplemented fallback, got {:#?}",
+        ability
+    );
+
+    let expected = ActivationRestriction::RequiresCondition {
+        condition: Some(ParsedCondition::And {
+            conditions: vec![
+                ParsedCondition::Not {
+                    condition: Box::new(ParsedCondition::IsYourTurn),
+                },
+                ParsedCondition::IsDuringUpkeep,
+            ],
+        }),
+    };
+    assert!(
+        ability.activation_restrictions.contains(&expected),
+        "expected composed opponent's-upkeep restriction, got {:?}",
+        ability.activation_restrictions
+    );
+}
+
 /// CR 508.1: a STANDALONE combat-window activation gate — "Activate only before
 /// attackers are declared" / "Activate only before combat" (Arcum's Whistle,
 /// Arcum's Sleigh) with no "during <role>" first half — must map to the enforced

crates/engine/src/types/ability.rs

@@ -7600,6 +7600,15 @@ pub enum ParsedCondition {
     /// ability-resolution layer), the same way `ParsedCondition::And` mirrors
     /// `AbilityCondition::And`.
     IsYourTurn,
+    /// CR 503.1: True when the game is currently in the upkeep step. A
+    /// turn-structure predicate with NO player scope — it asks only "is it an
+    /// upkeep step", not whose. Player scope composes at the restriction layer:
+    /// "during an opponent's upkeep" is `And([Not(IsYourTurn), IsDuringUpkeep])`
+    /// (CR 102.1 fixes the active player from the turn), mirroring how
+    /// opponent-turn activation timing reuses `Not(IsYourTurn)` (see
+    /// `opponents_turn_activation_condition`) rather than a dedicated
+    /// `DuringOpponents*` restriction sibling.
+    IsDuringUpkeep,
     /// CR 601.3d + CR 702.8a + CR 608.2c: The in-flight spell being cast targets at
     /// least one object that matches `filter`. Gates a target-dependent casting
     /// permission (Timely Ward — "you may cast this spell as though it had flash if

docs/parser-misparse-backlog.md

@@ -5071,7 +5071,7 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top
 
 </details>
 
-### 28. Trigger/activation timing or ordinal restriction dropped  (14 cards)
+### 28. Trigger/activation timing or ordinal restriction dropped  (13 cards)
 
 **Signature.** A timing/scope restriction (OnlyDuringYourTurn / OncePerTurn / 'during an opponent's turn' / Nth-spell ordinal / cast-timing) is null; the constraint tail is not parsed.
 
@@ -5090,7 +5090,6 @@ This is the prioritized "fix N root causes → unlock M cards" backlog: the top
 - Shadowheart, Sharran Cleric
 - Skarrgan Hellkite
 - Tomb Tyrant
-- Trade Caravan
 - Uthros Research Craft
 - Uthros, Titanic Godcore