Skip to content

fix(client): enforce colorless color identity for colorless commanders#5186

Open
jeffrey701 wants to merge 1 commit into
phase-rs:mainfrom
jeffrey701:fix/colorless-commander-identity
Open

fix(client): enforce colorless color identity for colorless commanders#5186
jeffrey701 wants to merge 1 commit into
phase-rs:mainfrom
jeffrey701:fix/colorless-commander-identity

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

Problem

isInColorIdentity (client/src/components/deck-builder/commanderUtils.ts) began with if (identity.length === 0) return true;. For a genuinely colorless commander (e.g. Kozilek, Butcher of Truth — color identity []), the combined identity is empty, so every card was treated as legal. A red card like Lightning Bolt passed the color-identity check, but CR 903.5c permits only colorless cards in a colorless commander's deck.

The subtlety (and why a naive fix is wrong): that same empty-identity short-circuit also absorbed the "commander card data hasn't loaded yet" case — a cache miss yields an empty identity too. Dropping the short-circuit outright would flag every colored card in the deck during the async card-data load window (false positives on a fresh open).

Fix

  • Remove the identity.length === 0 -> true short-circuit from isInColorIdentity; card.color_identity.every(c => set.has(c)) already does the right thing — [].every() is true (colorless card legal), a colored card is false (violation).
  • Move the loading guard to getColorIdentityViolations: return [] until every commander's data is cached, so a cache miss can't masquerade as "colorless commander."

Behavior for colored commanders and for the loading window is unchanged; only the genuinely-colorless-commander case is corrected.

Tests

Adds commanderUtils.test.ts: a colorless commander (Kozilek) now flags an off-color card and allows a colorless one (fails before the fix — no violation reported); a colored commander still flags only off-color cards; and an unloaded commander yields no violations (no false positives mid-load).

Self-contained: only commanderUtils.ts + a new test; no engine/Rust/protocol changes.

Model: claude-opus-4-8

isInColorIdentity short-circuited 'if (identity.length === 0) return true', so a genuinely colorless commander (e.g. Kozilek, color identity []) allowed any card — a red card passed as legal, violating CR 903.5c (a colorless commander permits only colorless cards). The short-circuit doubled as a 'commander data not loaded' guard, so simply dropping it would flag every colored card during the load window. Move that guard to getColorIdentityViolations (skip until all commanders are cached) and let [].every() enforce the colorless case correctly.
@jeffrey701 jeffrey701 requested a review from matthewevans as a code owner July 6, 2026 05:28
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@matthewevans matthewevans added the defer-fe Frontend/client/UI PR deferred to Matt's direct review label Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

defer-fe Frontend/client/UI PR deferred to Matt's direct review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants