ship/fix desktop show native engine provisioning status (#6546) - #9
Open
keloide wants to merge 14 commits into
Open
ship/fix desktop show native engine provisioning status (#6546)#9keloide wants to merge 14 commits into
keloide wants to merge 14 commits into
Conversation
Parse changes introduced by this PR · 28 card(s), 34 signature(s) (baseline: main
|
…ponent (phase-rs#6565) (phase-rs#6836) * test(engine): Riptide per-opponent target slots are scoped to that opponent's permanents (phase-rs#6565) The per-opponent fanout targeting fix (phase-rs#6565, "that player controls" bound to the iterated opponent) had no runtime regression covering the controller scope. The existing riptide_gearhulk_5994.rs tests cover the optional-count behavior (phase-rs#5994) but not that each opponent's slot targets THAT opponent's permanents rather than the caster's. Add two runtime tests driving Riptide's real ETB through the cast pipeline: - `riptide_per_opponent_slot_excludes_the_casters_own_permanents` offers only the caster's own permanent as target intent; it is not a legal target for the opponent's per-opponent slot, so the slot is declined and the permanent stays. Non-vacuous: a broken scope that let the opponent slot target the caster's permanent would move it to the library and flip the assertion. - `riptide_per_opponent_slot_targets_that_opponents_permanent` is the positive half — the opponent's own permanent IS legal and moves to its owner's library, while the caster's permanent is untouched. Refs phase-rs#6565 * test(PR-6836): cover both per-opponent target slots * fix(engine): place every per-opponent fanout target at its library position (phase-rs#6565) A per-opponent target fanout ("for each opponent, put up to one target nonland permanent that player controls into its owner's library third from the top") pre-selects one target PER opponent at stack time via multi_target.max = PlayerCount { Opponent }. The PutAtLibraryPosition effect's count field (Fixed(1)) is the PER-OPPONENT cap, not the total, so put_on_top::resolve wrongly treated the two pre-chosen permanents as "more candidates than count" and issued an interactive EffectZoneChoice { count: 1 } over the already-targeted permanents. That prompt looped forever (selecting one never advanced) and, at best, would have placed only one of the two. CR 601.2c: the number of targets is fixed at targeting; each chosen target is placed. CR 401.4: cards put at the same library position are arranged by their owner. For a per-opponent fanout, expected placement count is the number of pre-chosen targets, so each is routed to its own owner's library (CR 400.7). Fixes the runtime regression covered by riptide_per_opponent_slots_target_each_opponents_permanent (phase-rs#6836). --------- Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
* fix(parser): retain target for counter instead overrides * fix(parser): retain counter override event targets * fix(parser): bind instead to paid continuation --------- Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
…ated (phase-rs#6841) * fix(engine): stop basic-land mana fallback from bypassing CantBeActivated land_mana_options() fell back to unconditional subtype-inferred mana production whenever scan_mana_abilities() returned no options, without checking whether that emptiness meant "no mana ability exists" or "a real mana ability exists but was just filtered out by a legality gate." Karn, the Great Creator correctly filtered a Liquimetal-Coating-turned- artifact land's own {T}: Add ability out of scan_mana_abilities, but the fallback then silently re-added it via bare subtype inference, letting the opponent tap the blocked land for mana anyway (phase-rs#6469). Gate the fallback on the object genuinely carrying no Effect::Mana ability at all, so Urborg/Blood-Moon-class subtype-only production still works while a real, currently-prohibited ability stays prohibited. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(PR-6841): cite land-type mana fallback rule * fix(engine): block the bare-subtype-land mana fallback too The first fix only closed the gap for a land carrying an explicit Effect::Mana ability. A land with NO explicit ability at all - just a basic land subtype, the genuine Urborg/Blood-Moon-class case the fallback exists for - hits land_mana_options()'s bare-subtype branch directly, bypassing scan_mana_abilities() entirely and any CantBeActivated check with it. Per CR 305.6 that intrinsic "{T}: Add [mana symbol]" ability is still an activated mana ability, so CR 602.5 prohibitions must block it exactly like a printed one. Add mana_abilities::intrinsic_land_mana_ability_blocked, which builds a minimal synthetic AbilityDefinition for the intrinsic ability and delegates to the single-authority is_blocked_by_cant_be_activated / is_blocked_by_cant_activate_during checks - never re-implements them. Wire it into the fallback, gated on require_current_payability to match how is_active_tap_mana_ability treats real abilities (the auto-tap planning pass doesn't consult per-source legality gates for any mana source, real or intrinsic). Adds a regression test with a bare-subtype artifact land under Karn's prohibition (the case the prior fix's Forest test couldn't reach, since Forest carries an explicit ability) and a positive companion test confirming the ordinary bare-subtype fallback still works with no prohibition in play. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(engine): route the intrinsic land mana ability through the shared readiness authority intrinsic_land_mana_ability_blocked reimplemented only two of the checks a printed mana ability goes through (CantBeActivated, CantActivateDuring), missing phased-out (CR 702.26b), detained (CR 701.35a), and can't-tap (CR 701.26a + CR 508.1f) sources. A bare-subtype land in any of those states was still offered as a legal mana source, since the equivalent printed mana ability is excluded by mana_ability_ready_without_simulation_gated but the synthesized intrinsic ability was checked against a narrower, hand-picked subset. That readiness authority takes an AbilityDefinition by reference and never indexes obj.abilities, so a synthesized definition with no real storage slot is exactly as valid an input as a printed one - no refactor of the authority needed. Route the synthetic {T}: Add ability through it directly instead of re-implementing any subset of its checks, preserving the require_current_payability split that already keeps the auto-tap planning path (which doesn't consult per-source legality gates for any mana source, real or intrinsic) distinct from the interactive legal-action path this whole family targets. Adds three regression tests (detained, phased-out, can't-tap bare-subtype lands, none of which need a CantBeActivated static to demonstrate the gap) alongside the existing CantBeActivated and positive-fallback coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(PR-6841): correct activation-cost CR annotations --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
…yment (phase-rs#6840) * fix(ai): reject dominated Colorless convoke-family taps during ManaPayment Metallic Rebuke's {2}{U} Improvise cast could dead-end: mana_payment_actions offers a Colorless TapForConvoke marker for every Improvise/Waterbend/Convoke eligible permanent, with nothing preferring a dual-purpose permanent's native colored mana ability while a colored pip is still outstanding. Tapping such a permanent via the Colorless marker first permanently strands the pip, and the AI dead-ends in ManaPayment with no legal completion, hitting fallback_action's debug_assert (a release panic) via search.rs's documented "can_cast_object_now guarantees completability" invariant -- true for the pre-cast check, not for tap-channel execution order. Add convoke_native_tap_still_demanded (mana_colors.rs), the tap-channel sibling of tap_strands_demanded_color, reusing its existing color_is_demanded helper rather than duplicating it. Wire it into tactical_gate.rs's assess_candidate as a new TapForConvoke{Colorless} arm: reject the Colorless tap only when a sibling native-ability candidate for the same object could still pay the pending cast's outstanding colored demand -- zero-cost dominance, not a scoring preference, since the native ability can always still cover the trailing generic slot afterward. Four new tactical_gate.rs regression tests cover: the dominated-Colorless rejection, the sibling native tap staying allowed, the tap being allowed once colored demand is satisfied (not a blanket ban), and a permanent with no native colored option being unaffected (gate stays scoped to true dominance). Closes phase-rs#6837 * fix(ai): recognize a colored TapForConvoke as a dominating sibling too sibling_native_tap_pays_demand only handled TapLandForMana and ActivateAbility as siblings that could still cover an outstanding colored demand -- it missed that mana_payment_actions emits a colored TapForConvoke candidate alongside the Colorless one for CR 702.51a Convoke (not just Improvise/Waterbend, which are always colorless). A Convoke spell with a remaining colored pip and a matching-colored creature left the Colorless marker Allow, since the previous code never recognized the colored sibling on the same object as dominating it -- reintroducing the exact ManaPayment dead-end this fix targets, just via Convoke instead of Improvise. Add a TapForConvoke{same object_id} arm matching color_is_demanded on its own mana_type; Colorless naturally evaluates false so it can't spuriously match itself. New regression drives the real production candidate path (candidate_actions_broad -> mana_payment_actions) for a Convoke spell with a colored pip and a matching-colored creature, rather than a synthetic Improvise-only sibling: asserts the Colorless candidate is Reject while the real colored sibling stays Allow. Fails on the prior head. * fix(ai): only treat a tap-cost native mana ability as a dominating sibling sibling_native_tap_pays_demand's ActivateAbility arm gated the Colorless convoke-family marker off ANY same-object ability that could produce a demanded color, without checking whether that ability's own cost actually taps the permanent. The engine deliberately keeps tapless mana abilities (e.g. a sacrifice-based one) in the ManaPayment candidate set alongside a Colorless Improvise/Convoke tap on the same object -- they are not mutually exclusive (Colorless first, then the tapless ability, is a legal completing sequence). The prior code rejected the Colorless candidate solely because the tapless sibling existed, which can itself recreate the ManaPayment dead-end this fix targets, just in the opposite direction. Gate the ActivateAbility arm on the ability's own cost categories (AbilityCost::categories(), CR 118) containing CostCategory::TapsSelf, using the engine's existing cost-component authority rather than re-matching cost shapes by hand -- it already flattens Composite costs. Two new regressions drive the real production ManaPayment candidate set (candidate_actions_broad -> mana_payment_actions) for an Improvise-eligible artifact with a same-object mana ability: a tapless (Sacrifice-cost) one leaves the Colorless candidate Allow (fails on the prior head), and a tap-cost one still gates it to Reject (regression guard for the existing behavior). --------- Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
…tch race (phase-rs#6775) * fix(build): Windows-compat fixes for Tilt cmd parsing and Scryfall fetch race Tilt runs STRING cmds through cmd.exe on Windows, which mangles nested double-quotes before bash ever sees them; switch wasm/clippy/card-data to list-form cmd to bypass cmd.exe's string parsing entirely. scryfall_download()'s temp-file+rename relies on POSIX rename silently replacing a file another process has open; Windows can't do that, so the losing side of setup.sh's parallel Scryfall fetches hard-failed instead of harmlessly losing. Fall back to accepting the winner's file when rename fails. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(scripts): caller-supplied validator for Scryfall fetch + mv-failure coverage Resolves review feedback on Windows-compat Scryfall fetch: - scryfall_download/scryfall_fetch_bulk accept optional validator arg (default scryfall_validate_json); single authority applied at both accept points (fresh download and mv-failure destination reuse). - gen-scryfall-sets.sh passes a .data-aware validator, validates a cached SETS_FILE before the OUTPUT early-exit (invalid cache is deleted and refetched once), and gains SCRYFALL_SETS_FILE/OUTPUT/URL override seams for hermetic testing. - scryfall.test.ts: hermetic mv-failure fixture via function shadowing of curl/mv (cases 1-5b) plus validator-discrimination cases (6/6b); every recovery-branch case asserts a branch-unique marker. Co-Authored-By: Claude <noreply@anthropic.com> * fix(PR-6775): preserve validated Windows rename recovery * test(PR-6775): pin rename failure diagnostics --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: Matt Evans <1388610+matthewevans@users.noreply.github.com> Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
* Fix Diluvian Primordial * Fix Diluvian Primordial review findings * fix(engine): preserve Diluvian free-cast continuations
…a choice (phase-rs#6848) Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
* fix(ai): price delivered draw benefits exactly * fix(ai): reject lethal life-cost candidates * fix(engine): preserve delayed trigger firing provenance * fix(ai): choose Evoke only for proven utility * feat(ai): plan fetchland mana routes * fix(ai): improve mana, mulligan, and swarm decisions * fix(ai): certify Pact upkeep payment routes * style(engine): format swarm witness * style(ai): format rebased selection paths * fix(engine): complete delayed trigger provenance plumbing * fix(engine): align Warp delayed trigger helper * fix(tests): use supported scenario state setup * fix(engine): update delayed trigger test helper * fix(engine): address PR 6842 review findings * fix(engine): normalize delayed trigger loop provenance * fix(tests): make delivery and AI fixtures hermetic * fix(ai): address implementation review findings --------- Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
…et to the monarch (The Spear of Bashenga phase-rs#5249) (phase-rs#6162) * fix(engine): scope "attacks the monarch" triggers + bind the destroy target to the monarch (The Spear of Bashenga phase-rs#5249) The Spear of Bashenga: "Whenever equipped creature attacks the monarch, destroy target tapped nonland permanent that player controls." The parser's `parse_attack_target` had no " the monarch" arm, so the trigger degraded to a bare `TriggerMode::Attacks` with no attack-target scope — it fired on EVERY attack and prompted for a destroy target regardless of who was the monarch. Two-part fix (both class-level building blocks, not one-offs): 1. New `AttackTargetFilter::Monarch` — a Player-type attack whose defending player must currently hold the monarch designation. Because the monarch is a dynamic single-player identity (CR 725.1) the pure type matcher can't see it, so `attack_target_type_matches` treats `Monarch` as a Player attack and the identity check is applied statefully in `attack_target_matches` against `state.monarch` (both `Some` and equal; no monarch → never fires). Parser arm `tag(" the monarch")` in `parse_attack_target`. 2. The "that player controls" destroy target was mis-bound. "attacks the monarch ... that player controls" parsed with `controller: You` (the Spear's controller = the ATTACKER), so the destroy would have hit the attacker's own tapped permanents — a wrong-player-target bug worse than the original. `parse_referenced_player_phrase` (the antecedent binder feeding `DefendingPlayer`) had no "the monarch" arm, so "that player" never bound to the defender. Added `tag("the monarch")` there so "that player" binds to `ControllerRef::DefendingPlayer` (the monarch IS the attacked player, CR 508.1a + CR 725.1) — also covers "deals combat damage to the monarch ... that player" for the whole class. `AttackTargetFilter::Monarch` is additive; all exhaustive matches carry explicit arms (no wildcards). CR annotations (508.1a, 725.1) grep-verified. Tests (each revert-probed): - parser shape: "attacks the monarch" → `attack_target_filter = Monarch`, `valid_card` = the equipped-creature subject, and the destroy target is controlled by `DefendingPlayer` (guards the wrong-target fix), zero Unimplemented; - matcher unit test: monarch defender → matches; non-monarch → no; no monarch → no; - integration (real combat): attacking the monarch fires the trigger; attacking a non-monarch does NOT fire (the reported bug — disabling the stateful check makes this fire, confirmed); no monarch → no fire. Curse "attacks enchanted player" + Curse of Predation regressions confirm no antecedent-binding collateral. Full lib: 17065 passed. Integration: 3412 passed. Clippy clean. Fixes phase-rs#5249. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(PR-6162): repair monarch matcher test context --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: matthewevans <matthewevans@users.noreply.github.com>
…s-with replacements
The Adamant cycle parsed its "enters with a +1/+1 counter" clause as a
CR 614.1c replacement but dropped the sentence-initial "If <condition>, "
gate, leaving ReplacementDefinition.condition = None. The engine therefore
gave every copy of these creatures the counter regardless of what mana was
actually spent — silently-wrong behaviour that nothing flagged at runtime.
Coverage reported it as Swallow:Condition_If on six cards.
Root cause: oracle_replacement.rs peeled a TRAILING " unless <cond>" gate
(extract_enters_with_unless_suffix) and a trailing " if <cond>" gate
(extract_enters_with_only_if_suffix), but nothing peeled a LEADING one, and
the payload scan skips everything before "enters with".
Fixed for the class, not the cards:
* extract_enters_with_leading_if_gate mirrors the existing trailing
extractor one-for-one, including its fail-closed tri-state. Condition
recognition delegates to parse_inner_condition (the single authority), so
the ENTIRE static-condition grammar is now reachable from the
sentence-initial position, where previously zero shapes were. An
unrecognised gate returns Unparsed and the clause falls through to
Effect::unimplemented rather than committing a replacement with the gate
erased.
* CR 207.2c ability-word labels are peeled via parse_known_ability_word_name,
which enumerates exactly the CR 207.2c list. CR 207.2d flavour words are
deliberately NOT peeled: the loose 4-word heuristic would peel "Sensational
Save" and regress Scarlet Spider, Ben Reilly to Unparsed.
* CastManaSpentMetric::OfColor parameterises the existing metric axis
(Total | DistinctColors | FromSource) rather than adding a
ReplacementCondition sibling, so the parsed condition flows through the
untouched replacement_condition_from_static into OnlyIfQuantity. Zero new
ReplacementCondition variants, zero new condition-evaluation arms.
* parse_color_mana_spent_threshold reuses parse_amount_threshold, so the
comparator axis (at least N / N or more / N or fewer) is carried rather
than hardcoded to GE.
Coverage: six cards flip to supported (Ardenvale, Embereth, Garenbrig,
Locthwain and Vantress Paladin, plus Necromantic Summons), zero cards
regress. Dust Animus keeps supported=true and gains its gate, so it is
correctly conditional for the first time. Henge Walker ("mana of the same
color" — a max-over-colors metric) and Red and Black Legacy now fail closed
and stay honestly red instead of silently granting their counters.
Measured against a pre-change baseline of all 35516 cards: 0 true->false,
6 false->true, net +6.
CR 614.1c, CR 614.12, CR 207.2c, CR 207.2d, CR 106.3, CR 601.2h, CR 400.7d.
CR 603.4 is deliberately not cited in code: its intervening-if rule is
scoped to triggered abilities, and this gates a replacement effect.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Silverflame Ritual's Adamant rider grants a continuous static effect
("creatures you control gain vigilance") rather than an ability-level
effect, so its gate lands on `StaticDefinition.condition` (CR 613 layer
6) instead of the ability's own `condition`. The CI parse-diff bot reads
the ability level only, so that layer move renders as
`conditional: 3+ White spent -> ∅`, which reads like a dropped gate --
the exact bug class this file guards.
It is not a drop: the condition is present and enforced. Verified at
runtime in both polarities, because nothing pinned this subclass. The
existing R8 rider guard (Slaying Fire) cannot catch it -- that payload
is ability-level -- so a future refactor really could drop the static's
condition with every other test here staying green.
Both new tests carry a positive reach-guard asserting the card's UNGATED
first line resolved (the +1/+1 counter), so the "no vigilance" negative
cannot pass vacuously on a spell that never resolved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
matthewevans
force-pushed
the
claude/phase-developer-track-card-o1e67k
branch
from
August 1, 2026 17:27
7c931d6 to
9f77865
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(desktop): show native engine provisioning status
fix(release): recover main bundle and clarify desktop downloads
Co-authored-by: matthewevans matthewevans@users.noreply.github.com