From a3ba5670dbdf0217b7821f2d1b5d2f71823adf03 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Sun, 9 Aug 2026 00:46:26 -0700 Subject: [PATCH 1/3] fix(client): address sandbox and sync review findings --- client/src/adapter/types.ts | 13 ++++++- .../chrome/DebugCardContextMenu.tsx | 3 +- .../components/chrome/DebugObjectActions.tsx | 6 +-- .../components/settings/PreferencesModal.tsx | 6 +++ client/src/game/__tests__/diceContest.test.ts | 27 ++++++++++++- .../src/game/__tests__/sessionCleanup.test.ts | 11 ++++++ client/src/game/dispatch.ts | 3 +- client/src/game/sessionCleanup.ts | 1 + client/src/i18n/locales/de/game.json | 1 + client/src/i18n/locales/en/game.json | 1 + client/src/i18n/locales/es/game.json | 1 + client/src/i18n/locales/fr/game.json | 1 + client/src/i18n/locales/it/game.json | 1 + client/src/i18n/locales/pl/game.json | 1 + client/src/i18n/locales/pt/game.json | 1 + client/src/services/__tests__/backup.test.ts | 14 +++++++ client/src/services/backup.ts | 39 +++++++++++++++---- 17 files changed, 115 insertions(+), 15 deletions(-) diff --git a/client/src/adapter/types.ts b/client/src/adapter/types.ts index 9983fe29f6..bb36bdaccd 100644 --- a/client/src/adapter/types.ts +++ b/client/src/adapter/types.ts @@ -2424,6 +2424,17 @@ export type PlanarDieFace = "Planeswalk" | "Chaos" | "Blank"; // ── Game Events (discriminated union, tag="type", content="data") ──────── +/** Exact serde spellings of the engine's `PlayerActionKind` enum. */ +export type PlayerActionKind = + | "AcceptedOptionalEffect" + | "SearchedLibrary" + | "Scry" + | "Surveil" + | "CollectEvidence" + | "ShuffledLibrary" + | "Proliferate" + | "Investigate"; + export type GameEvent = | { type: "GameStarted" } | { @@ -2470,7 +2481,7 @@ export type GameEvent = type: "PlayerPerformedAction"; data: { player_id: PlayerId; - action: string; + action: PlayerActionKind; look_count?: number; scry_bottom_count?: number; scry_top_count?: number; diff --git a/client/src/components/chrome/DebugCardContextMenu.tsx b/client/src/components/chrome/DebugCardContextMenu.tsx index 62153275e6..fe0ad0a61b 100644 --- a/client/src/components/chrome/DebugCardContextMenu.tsx +++ b/client/src/components/chrome/DebugCardContextMenu.tsx @@ -8,7 +8,6 @@ import type { ObjectId, Zone, } from "../../adapter/types"; -import { formatCounterType } from "../../viewmodel/cardProps"; import { useGameStore } from "../../stores/gameStore"; import { useUiStore } from "../../stores/uiStore"; import { useGameDispatch } from "../../hooks/useGameDispatch"; @@ -252,7 +251,7 @@ function DebugCardContextMenuInner({ ? [ (null); const [owner, setOwner] = useState(0); const [nonlegendary, setNonlegendary] = useState(false); @@ -152,7 +153,7 @@ function CreateTokenCopyForm({ onDispatch }: Props) { - + @@ -228,7 +229,6 @@ function ModifyCountersForm({ onDispatch }: Props) { value={counterType} onChange={setCounterType} options={counterTypes} - getOptionLabel={formatCounterType} /> diff --git a/client/src/components/settings/PreferencesModal.tsx b/client/src/components/settings/PreferencesModal.tsx index 89c682a38b..82241b5c32 100644 --- a/client/src/components/settings/PreferencesModal.tsx +++ b/client/src/components/settings/PreferencesModal.tsx @@ -1014,6 +1014,12 @@ function CloudSyncSection() { > {t("sync.keepLocal")} + ) : ( diff --git a/client/src/game/__tests__/diceContest.test.ts b/client/src/game/__tests__/diceContest.test.ts index 4b6041114d..240ee7f9be 100644 --- a/client/src/game/__tests__/diceContest.test.ts +++ b/client/src/game/__tests__/diceContest.test.ts @@ -1,8 +1,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { GameEvent } from "../../adapter/types"; +import type { GameAction, GameEvent, SubmitResult } from "../../adapter/types"; +import { useGameStore } from "../../stores/gameStore"; import { usePreferencesStore } from "../../stores/preferencesStore"; import { useUiStore } from "../../stores/uiStore"; +import { buildEngineAdapterMock } from "../../test/factories/engineAdapterFactory"; +import { buildGameState } from "../../test/factories/gameStateFactory"; +import { dispatchAction } from "../dispatch"; import { flashCompletedScry, flashInGameRolls, flashStartingPlayerContest } from "../diceContest"; const die = (player_id: number, sides: number, result: number): GameEvent => ({ @@ -33,6 +37,7 @@ const scry = (player_id: number, top: number, bottom: number): GameEvent => ({ beforeEach(() => { vi.useFakeTimers(); + useGameStore.getState().reset(); usePreferencesStore.setState({ animationSpeedMultiplier: 1 }); useUiStore.setState({ diceRoll: null, diceRollQueue: [] }); useUiStore.getState().resetScryOutcome(); @@ -158,6 +163,26 @@ describe("flashCompletedScry", () => { expect(useUiStore.getState().scryOutcome).toBeNull(); }); + + it("is invoked by the production dispatch pipeline", async () => { + usePreferencesStore.setState({ animationSpeedMultiplier: 0 }); + const state = buildGameState({ stack: [], players: [] }); + const adapter = buildEngineAdapterMock(state, { + submitAction: vi.fn().mockResolvedValue({ + events: [scry(0, 2, 1)], + log_entries: [], + } satisfies SubmitResult), + }); + useGameStore.setState({ adapter, gameState: state, gameMode: "ai" }); + + await dispatchAction({ type: "PassPriority" } as GameAction, 0); + + expect(useUiStore.getState().scryOutcome).toEqual({ + playerId: 0, + topCount: 2, + bottomCount: 1, + }); + }); }); describe("flashInGameRolls", () => { diff --git a/client/src/game/__tests__/sessionCleanup.test.ts b/client/src/game/__tests__/sessionCleanup.test.ts index 2fb389da7e..bdb527cbaf 100644 --- a/client/src/game/__tests__/sessionCleanup.test.ts +++ b/client/src/game/__tests__/sessionCleanup.test.ts @@ -13,6 +13,7 @@ describe("clearPromptOverlayState", () => { enchantmentsDialogPlayer: null, manualManaOverride: false, mobileHandGesture: null, + scryOutcome: null, }); }); @@ -108,4 +109,14 @@ describe("clearPromptOverlayState", () => { expect(useUiStore.getState().diceRoll).toBeNull(); expect(useUiStore.getState().diceRollQueue).toEqual([]); }); + + it("clears a completed scry overlay at a game boundary", () => { + useUiStore.setState({ + scryOutcome: { playerId: 1, topCount: 2, bottomCount: 1 }, + }); + + clearPromptOverlayState(); + + expect(useUiStore.getState().scryOutcome).toBeNull(); + }); }); diff --git a/client/src/game/dispatch.ts b/client/src/game/dispatch.ts index 6a54c86d6f..5c0c69718c 100644 --- a/client/src/game/dispatch.ts +++ b/client/src/game/dispatch.ts @@ -9,7 +9,7 @@ import type { AnimationStep } from "../animation/types"; import { audioManager } from "../audio/AudioManager"; import { MAX_UNDO_HISTORY, UNDOABLE_ACTIONS } from "../constants/game"; import { debugLog } from "./debugLog"; -import { flashInGameRolls } from "./diceContest"; +import { flashCompletedScry, flashInGameRolls } from "./diceContest"; import i18n from "../i18n"; import { useAnimationStore } from "../stores/animationStore"; import { useAppNotificationStore } from "../stores/appToastStore"; @@ -505,6 +505,7 @@ async function processAction( // way the turn banner bypasses the animation queue. These events are marked // NON_VISUAL so normalizeEvents skips them below. flashInGameRolls(events); + flashCompletedScry(events); // 6. Normalize events into animation steps const pacingMultipliers = usePreferencesStore.getState().pacingMultipliers; diff --git a/client/src/game/sessionCleanup.ts b/client/src/game/sessionCleanup.ts index d68c71f372..ebcd496145 100644 --- a/client/src/game/sessionCleanup.ts +++ b/client/src/game/sessionCleanup.ts @@ -31,6 +31,7 @@ export function clearPromptOverlayState(): void { useUiStore.getState().setAttachmentFanHost(null); useUiStore.getState().setMobileHandGesture(null); useUiStore.getState().resetDiceRoll(); + useUiStore.getState().resetScryOutcome(); // The per-game "Manual mana" toggle must never leak into the next game. useUiStore.getState().setManualManaOverride(false); // The ephemeral hand hide-filter is a per-game focus aid — reset it too. diff --git a/client/src/i18n/locales/de/game.json b/client/src/i18n/locales/de/game.json index d759ab3937..75bf6f851f 100644 --- a/client/src/i18n/locales/de/game.json +++ b/client/src/i18n/locales/de/game.json @@ -45,6 +45,7 @@ "expand": "Auflösungsfortschritt ausklappen" }, "debugCreate": { + "copies": "Kopien", "tokenPower": "Power", "tokenToughness": "Toughness", "tokenPowerPlaceholder": "Power", diff --git a/client/src/i18n/locales/en/game.json b/client/src/i18n/locales/en/game.json index 2a65bf4c3e..6ac2b8897a 100644 --- a/client/src/i18n/locales/en/game.json +++ b/client/src/i18n/locales/en/game.json @@ -51,6 +51,7 @@ "expand": "Expand resolving progress" }, "debugCreate": { + "copies": "Copies", "tokenPower": "Power", "tokenToughness": "Toughness", "tokenPowerPlaceholder": "Power", diff --git a/client/src/i18n/locales/es/game.json b/client/src/i18n/locales/es/game.json index 917ced0642..40f56d06d8 100644 --- a/client/src/i18n/locales/es/game.json +++ b/client/src/i18n/locales/es/game.json @@ -45,6 +45,7 @@ "expand": "Expandir progreso de resolución" }, "debugCreate": { + "copies": "Copias", "tokenPower": "Power", "tokenToughness": "Toughness", "tokenPowerPlaceholder": "Power", diff --git a/client/src/i18n/locales/fr/game.json b/client/src/i18n/locales/fr/game.json index 7be0fc91f6..f755581635 100644 --- a/client/src/i18n/locales/fr/game.json +++ b/client/src/i18n/locales/fr/game.json @@ -45,6 +45,7 @@ "expand": "Développer la progression de résolution" }, "debugCreate": { + "copies": "Copies", "tokenPower": "Power", "tokenToughness": "Toughness", "tokenPowerPlaceholder": "Power", diff --git a/client/src/i18n/locales/it/game.json b/client/src/i18n/locales/it/game.json index 77fc9b2871..505b456aa5 100644 --- a/client/src/i18n/locales/it/game.json +++ b/client/src/i18n/locales/it/game.json @@ -45,6 +45,7 @@ "expand": "Espandi avanzamento risoluzione" }, "debugCreate": { + "copies": "Copie", "tokenPower": "Power", "tokenToughness": "Toughness", "tokenPowerPlaceholder": "Power", diff --git a/client/src/i18n/locales/pl/game.json b/client/src/i18n/locales/pl/game.json index be3308e13d..d5a07fd315 100644 --- a/client/src/i18n/locales/pl/game.json +++ b/client/src/i18n/locales/pl/game.json @@ -45,6 +45,7 @@ "expand": "Rozwiń postęp rozwiązywania" }, "debugCreate": { + "copies": "Kopie", "tokenPower": "Power", "tokenToughness": "Toughness", "tokenPowerPlaceholder": "Power", diff --git a/client/src/i18n/locales/pt/game.json b/client/src/i18n/locales/pt/game.json index 17dd8a5389..fe9a72f37d 100644 --- a/client/src/i18n/locales/pt/game.json +++ b/client/src/i18n/locales/pt/game.json @@ -45,6 +45,7 @@ "expand": "Expandir progresso de resolução" }, "debugCreate": { + "copies": "Cópias", "tokenPower": "Power", "tokenToughness": "Toughness", "tokenPowerPlaceholder": "Power", diff --git a/client/src/services/__tests__/backup.test.ts b/client/src/services/__tests__/backup.test.ts index 4a8b5cd2e4..970cfee82f 100644 --- a/client/src/services/__tests__/backup.test.ts +++ b/client/src/services/__tests__/backup.test.ts @@ -131,4 +131,18 @@ describe("mergeDeckCollections", () => { { id: "cloud-folder", name: "Cloud", order: 0 }, ]); }); + + it("does not merge malformed cloud folder or deck metadata entries", () => { + const local = backup({ Local: "local" }); + local.deckMetadata = JSON.stringify({ Local: { addedAt: 1 } }); + local.deckFolders = JSON.stringify([{ id: "local-folder", name: "Local", order: 0 }]); + const cloud = backup({ Remote: "remote" }); + cloud.deckMetadata = JSON.stringify({ Remote: null }); + cloud.deckFolders = JSON.stringify([{ id: 42, name: "Invalid", order: 0 }]); + + const merged = mergeDeckCollections(local, cloud); + + expect(merged.deckMetadata).toBe(local.deckMetadata); + expect(merged.deckFolders).toBe(local.deckFolders); + }); }); diff --git a/client/src/services/backup.ts b/client/src/services/backup.ts index 72e7d7b384..145f51633d 100644 --- a/client/src/services/backup.ts +++ b/client/src/services/backup.ts @@ -108,23 +108,48 @@ export function mergeDeckCollections( }; } -function parseRecord(raw: string | null): Record | null { +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function parseRecord( + raw: string | null, + isValue?: (value: unknown) => value is T, +): Record | null { if (raw == null) return {}; try { const value: unknown = JSON.parse(raw); - return value !== null && typeof value === "object" && !Array.isArray(value) - ? (value as Record) - : null; + if (!isRecord(value)) return null; + if (isValue && !Object.values(value).every(isValue)) return null; + return value as Record; } catch { return null; } } +function isDeckMeta(value: unknown): value is DeckMeta { + if (!isRecord(value) || typeof value.addedAt !== "number") return false; + return ( + (value.lastPlayedAt === undefined || typeof value.lastPlayedAt === "number") && + (value.folderId === undefined || typeof value.folderId === "string") && + (value.starred === undefined || typeof value.starred === "boolean") + ); +} + +function isDeckFolder(value: unknown): value is DeckFolder { + return ( + isRecord(value) && + typeof value.id === "string" && + typeof value.name === "string" && + typeof value.order === "number" + ); +} + function parseFolders(raw: string | null | undefined): DeckFolder[] | null { if (raw == null) return []; try { const value: unknown = JSON.parse(raw); - return Array.isArray(value) ? (value as DeckFolder[]) : null; + return Array.isArray(value) && value.every(isDeckFolder) ? value : null; } catch { return null; } @@ -168,8 +193,8 @@ function mergeDeckMetadata( cloudDeckNames: ReadonlyMap, folderIds: ReadonlyMap, ): string | null { - const local = parseRecord(localRaw); - const cloud = parseRecord(cloudRaw); + const local = parseRecord(localRaw, isDeckMeta); + const cloud = parseRecord(cloudRaw, isDeckMeta); if (local === null || cloud === null) return localRaw; for (const [name, meta] of Object.entries(cloud)) { From f2dcbe8ea7a2f429e34171e45691dfa52ee07926 Mon Sep 17 00:00:00 2001 From: matthewevans Date: Sun, 9 Aug 2026 01:54:38 -0700 Subject: [PATCH 2/3] fix(debug): make card creation no-ops transactional --- .../__tests__/p2p-adapter-multiplayer.test.ts | 114 +++++++ .../adapter/__tests__/wasm-adapter.test.ts | 60 ++++ .../src/adapter/__tests__/ws-adapter.test.ts | 30 ++ client/src/adapter/engine-worker.ts | 12 - client/src/adapter/p2p-adapter.ts | 26 ++ client/src/adapter/wasm-adapter.ts | 24 +- client/src/adapter/ws-adapter.ts | 13 +- .../components/chrome/DebugCreateActions.tsx | 11 +- client/src/network/__tests__/protocol.test.ts | 3 +- client/src/network/protocol.ts | 11 +- crates/engine-wasm/src/lib.rs | 232 ++++++++++---- crates/engine/src/game/engine.rs | 58 +++- crates/engine/src/game/engine_debug.rs | 289 +++++++++++++++--- crates/engine/src/game/mod.rs | 4 +- crates/lobby-broker/src/protocol.rs | 11 +- crates/phase-server/src/main.rs | 40 +++ crates/server-core/src/protocol.rs | 24 +- crates/server-core/src/session.rs | 246 ++++++++++++--- scripts/check-protocol-version.mjs | 2 +- 19 files changed, 1012 insertions(+), 198 deletions(-) diff --git a/client/src/adapter/__tests__/p2p-adapter-multiplayer.test.ts b/client/src/adapter/__tests__/p2p-adapter-multiplayer.test.ts index f39ab4b1c4..ea7a042232 100644 --- a/client/src/adapter/__tests__/p2p-adapter-multiplayer.test.ts +++ b/client/src/adapter/__tests__/p2p-adapter-multiplayer.test.ts @@ -1131,6 +1131,69 @@ describe("P2PHostAdapter — 3-4p multiplayer", () => { expect(mockGetViewerSnapshot).toHaveBeenCalledWith(2); }); + it("keeps a host zero-count debug create out of transition side effects", async () => { + const { adapter } = makeHost(2); + await adapter.initialize(); + const revisionBefore = (adapter as unknown as { authoritativeRevision: number }) + .authoritativeRevision; + + await expect(adapter.submitAction({ + type: "Debug", + data: { + type: "CreateCard", + data: { + card_name: "Lightning Bolt", + owner: 0, + zone: "Hand", + run_etb: false, + nonlegendary: false, + count: 0, + }, + }, + }, 0)).resolves.toEqual({ events: [] }); + + expect(mockSubmitAction).toHaveBeenCalledOnce(); + expect((adapter as unknown as { authoritativeRevision: number }).authoritativeRevision) + .toBe(revisionBefore); + expect(mockGetViewerSnapshot).not.toHaveBeenCalled(); + expect(mockGetState).not.toHaveBeenCalled(); + }); + + it("acknowledges a guest zero-count debug create without broadcasting a transition", async () => { + const { adapter, emitConnection } = makeHost(2); + await adapter.initialize(); + const guest = await joinGuest(emitConnection, { + type: "guest_deck", + deckData: { player: { main_deck: [], sideboard: [] } }, + }); + await adapter.initializeGame(); + guest.sent.length = 0; + mockGetViewerSnapshot.mockClear(); + mockGetState.mockClear(); + const revisionBefore = (adapter as unknown as { authoritativeRevision: number }) + .authoritativeRevision; + + await guest.simulateData({ + type: "action", + senderPlayerId: 1, + action: { + type: "Debug", + data: { + type: "CreateTokenCopy", + data: { source_id: 1, owner: 1, nonlegendary: false, count: 0 }, + }, + }, + }); + + expect(await guest.getSentMessages()).toEqual([ + expect.objectContaining({ type: "action_noop" }), + ]); + expect((adapter as unknown as { authoritativeRevision: number }).authoritativeRevision) + .toBe(revisionBefore); + expect(mockGetViewerSnapshot).not.toHaveBeenCalled(); + expect(mockGetState).not.toHaveBeenCalled(); + }); + it("holds the seat on guest disconnect and NEVER auto-concedes on grace expiry", async () => { const { adapter, emitConnection } = makeHost(3, 5_000); await adapter.initialize(); @@ -1772,6 +1835,57 @@ describe("P2PHostAdapter — 3-4p multiplayer", () => { ); }); + it("guest receive path resolves action_noop without replacing its cached snapshot", async () => { + const { peer } = createFakePeer(); + const conn = new FakeDataConnection(); + const adapter = new P2PGuestAdapter( + { player: { main_deck: [], sideboard: [] } }, + peer as unknown as Peer, + "host-peer", + conn as unknown as DataConnection, + ); + const emitted = vi.fn(); + adapter.onEvent(emitted); + await adapter.initialize(); + const setupState = remoteState("setup"); + await conn.simulateData({ + type: "game_setup", + wireProtocolVersion: WIRE_PROTOCOL_VERSION, + assignedPlayerId: 1, + playerToken: "seat-token", + state: setupState, + events: [], + legalActions: [], + autoPassRecommended: false, + manaPaymentShortcutActions: [], + }); + await adapter.initializeGame(); + const cachedSnapshot = await adapter.getSnapshot(); + emitted.mockClear(); + + const pending = adapter.submitAction({ + type: "Debug", + data: { + type: "CreateCard", + data: { + card_name: "Lightning Bolt", + owner: 1, + zone: "Hand", + run_etb: false, + nonlegendary: false, + count: 0, + }, + }, + }, 1); + await conn.simulateData({ type: "action_noop" }); + + await expect(pending).resolves.toEqual({ events: [], log_entries: [] }); + expect(await adapter.getSnapshot()).toBe(cachedSnapshot); + expect(emitted).not.toHaveBeenCalledWith( + expect.objectContaining({ type: "stateChanged" }), + ); + }); + // Issue #5913: the host relays the engine's verdict verbatim, so a guest must // classify a stale ReorderHand exactly as the local-WASM seat does. Before the // shared classifier this path built a generic ACTION_REJECTED, and diff --git a/client/src/adapter/__tests__/wasm-adapter.test.ts b/client/src/adapter/__tests__/wasm-adapter.test.ts index 4396a08b3a..111282c316 100644 --- a/client/src/adapter/__tests__/wasm-adapter.test.ts +++ b/client/src/adapter/__tests__/wasm-adapter.test.ts @@ -380,6 +380,21 @@ describe("WasmAdapter", () => { }); describe("submitAction", () => { + const createCard = (count: number) => ({ + type: "Debug" as const, + data: { + type: "CreateCard" as const, + data: { + card_name: "Lightning Bolt", + owner: 0, + zone: "Hand" as const, + run_etb: false, + nonlegendary: false, + count, + }, + }, + }); + it("throws AdapterError with NOT_INITIALIZED if not initialized", async () => { await expect( adapter.submitAction({ type: "PassPriority" }, 0), @@ -404,6 +419,51 @@ describe("WasmAdapter", () => { ); }); + it("submits a zero-count debug create without loading the card database", async () => { + await adapter.initialize(); + + await expect(adapter.submitAction(createCard(0), 0)).resolves.toEqual({ + events: [], + log_entries: [], + }); + + expect(mockWorkerClient.submitAction).toHaveBeenCalledOnce(); + expect(mockWorkerClient.loadCardDbFromUrl).not.toHaveBeenCalled(); + }); + + it("does not load the card database when Rust rejects debug-create preflight", async () => { + mockWorkerClient.submitAction.mockRejectedValueOnce( + new Error("Engine error: DebugAction is only allowed in Sandbox mode"), + ); + await adapter.initialize(); + + await expect(adapter.submitAction(createCard(1), 0)).rejects.toThrow( + "DebugAction is only allowed in Sandbox mode", + ); + + expect(mockWorkerClient.submitAction).toHaveBeenCalledOnce(); + expect(mockWorkerClient.loadCardDbFromUrl).not.toHaveBeenCalled(); + }); + + it("loads the card database and retries only after Rust admits a nonzero create", async () => { + mockWorkerClient.submitAction + .mockRejectedValueOnce(new Error("Engine error: card database not loaded")) + .mockResolvedValueOnce({ events: [], log_entries: [] }); + await adapter.initialize(); + + await expect(adapter.submitAction(createCard(1), 0)).resolves.toEqual({ + events: [], + log_entries: [], + }); + + expect(mockWorkerClient.submitAction).toHaveBeenCalledTimes(2); + expect(mockWorkerClient.loadCardDbFromUrl).toHaveBeenCalledOnce(); + expect(mockWorkerClient.submitAction.mock.invocationCallOrder[0]) + .toBeLessThan(mockWorkerClient.loadCardDbFromUrl.mock.invocationCallOrder[0]); + expect(mockWorkerClient.loadCardDbFromUrl.mock.invocationCallOrder[0]) + .toBeLessThan(mockWorkerClient.submitAction.mock.invocationCallOrder[1]); + }); + // Regression: state-loss classification splits on whether the panic // hook captured a message. ENGINE_PANIC must NOT be retried (re-running // the same input re-panics — the user-reported "ai-getAction-retry" diff --git a/client/src/adapter/__tests__/ws-adapter.test.ts b/client/src/adapter/__tests__/ws-adapter.test.ts index 873e5d2d50..c82641d0d3 100644 --- a/client/src/adapter/__tests__/ws-adapter.test.ts +++ b/client/src/adapter/__tests__/ws-adapter.test.ts @@ -928,6 +928,36 @@ describe("WebSocketAdapter", () => { }); }); + it("resolves an accepted no-op without publishing a state transition", async () => { + const listener = vi.fn(); + adapter.onEvent(listener); + const pending = adapter.submitAction( + { + type: "Debug", + data: { + type: "CreateCard", + data: { + card_name: "Lightning Bolt", + owner: 0, + zone: "Hand", + run_etb: false, + nonlegendary: false, + count: 0, + }, + }, + }, + 0, + ); + + ws.dispatchSynthetic("message", JSON.stringify({ type: "ActionNoOp" })); + + await expect(pending).resolves.toEqual({ events: [], log_entries: [] }); + expect(listener).toHaveBeenCalledWith({ type: "actionPendingChanged", pending: false }); + expect(listener).not.toHaveBeenCalledWith( + expect.objectContaining({ type: "stateChanged" }), + ); + }); + // A refused takeback answers a fire-and-forget request, so no promise owns // the rejection. Before this branch the whole `if (this.pendingReject)` // body was skipped and the refusal was dropped on the floor — which is why diff --git a/client/src/adapter/engine-worker.ts b/client/src/adapter/engine-worker.ts index 9145321a72..c5c59f8a52 100644 --- a/client/src/adapter/engine-worker.ts +++ b/client/src/adapter/engine-worker.ts @@ -251,18 +251,6 @@ self.onmessage = async (e: MessageEvent) => { } case "submitAction": { - if ( - !cardDbLoaded && - msg.action?.type === "Debug" && - msg.action?.data?.type === "CreateCard" - ) { - const resp = await fetch(__CARD_DATA_URL__); - if (resp.ok) { - const text = await resp.text(); - load_card_database(text); - cardDbLoaded = true; - } - } const actionResult = submit_action(msg.actor, msg.action); if (typeof actionResult === "string") { // Rust's submit_action error contract: returns the error string diff --git a/client/src/adapter/p2p-adapter.ts b/client/src/adapter/p2p-adapter.ts index 8dcda48411..07b69804a5 100644 --- a/client/src/adapter/p2p-adapter.ts +++ b/client/src/adapter/p2p-adapter.ts @@ -567,6 +567,18 @@ function traceAdapter(side: "Host" | "Guest", event: string, data?: Record { this.assertInitialized(); - if (action.type === "Debug" && action.data.type === "CreateCard") { - await this.ensureCardDb(); - } try { - const result = this.engine ? await this.engine.submitAction(actor, action) : await this.fallback!.submitAction(action, actor); + const submit = () => this.engine + ? this.engine.submitAction(actor, action) + : this.fallback!.submitAction(action, actor); + let result: SubmitResult; + try { + result = await submit(); + } catch (error) { + if (!isDebugCreateCard(action) || !isDebugCreateCardDbMissing(error)) throw error; + await this.ensureCardDb(); + result = await submit(); + } this.invalidateAiDecisionDiagnostics(); return result; } catch (err) { diff --git a/client/src/adapter/ws-adapter.ts b/client/src/adapter/ws-adapter.ts index 7ea6557644..7fe9e6959d 100644 --- a/client/src/adapter/ws-adapter.ts +++ b/client/src/adapter/ws-adapter.ts @@ -202,6 +202,7 @@ export class NativeEngineVersionMismatchError extends Error { * `crates/server-core/src/protocol.rs`. Bump in lockstep when either side * adds, removes, renames, or changes the type of a protocol variant field. * + * 26 — Added ActionNoOp acknowledgement for accepted transport no-ops. * 25 — DebugCardEntries added a serialized, private resolution frame for * multi-card sandbox battlefield entries that pause for replacement or * as-enters choices. Old peers cannot deserialize that GameState shape. @@ -231,7 +232,7 @@ export class NativeEngineVersionMismatchError extends Error { * into a MulliganDecisionPhase::BottomCards sub-phase on * WaitingFor::MulliganDecision. */ -export const PROTOCOL_VERSION = 25; +export const PROTOCOL_VERSION = 26; /** * Lowest server protocol version this client will accept in the handshake. @@ -1449,6 +1450,16 @@ export class WebSocketAdapter implements EngineAdapter { break; } + case "ActionNoOp": { + this.emit({ type: "actionPendingChanged", pending: false }); + if (this.pendingResolve) { + this.pendingResolve({ events: [], log_entries: [] }); + this.pendingResolve = null; + this.pendingReject = null; + } + break; + } + case "ManaPaymentPreview": { const data = msg.data as { request_id: number; source_ids: ObjectId[] }; const pending = this.pendingManaPaymentPreviews.get(data.request_id); diff --git a/client/src/components/chrome/DebugCreateActions.tsx b/client/src/components/chrome/DebugCreateActions.tsx index 5521f6ae0e..478cf5b138 100644 --- a/client/src/components/chrome/DebugCreateActions.tsx +++ b/client/src/components/chrome/DebugCreateActions.tsx @@ -147,6 +147,7 @@ interface CardFaceShape { } function CreateCardForm({ onDispatch }: Props) { + const { t } = useTranslation("game"); const [cardName, setCardName] = useState(""); const [owner, setOwner] = useState(0); const [zone, setZone] = useState("Hand"); @@ -232,7 +233,7 @@ function CreateCardForm({ onDispatch }: Props) { - + {showAttachPicker && ( @@ -562,7 +563,7 @@ function CatalogTokenForm({ onDispatch }: Props) { - +
@@ -651,6 +652,7 @@ function CatalogTokenForm({ onDispatch }: Props) { } function CustomTokenForm({ onDispatch }: Props) { + const { t } = useTranslation("game"); const [name, setName] = useState(""); const [owner, setOwner] = useState(0); const [power, setPower] = useState(1); @@ -728,7 +730,7 @@ function CustomTokenForm({ onDispatch }: Props) { - + @@ -794,6 +796,7 @@ function CustomTokenForm({ onDispatch }: Props) { // copiable-value snapshotting, legendary-rule SBAs, ETB triggers — so this // form is a thin source+owner picker over the `CreateTokenCopy` debug action. function CopyPermanentForm({ onDispatch }: Props) { + const { t } = useTranslation("game"); const [sourceId, setSourceId] = useState(null); const [owner, setOwner] = useState(0); const [nonlegendary, setNonlegendary] = useState(false); @@ -813,7 +816,7 @@ function CopyPermanentForm({ onDispatch }: Props) { - + diff --git a/client/src/network/__tests__/protocol.test.ts b/client/src/network/__tests__/protocol.test.ts index d2d422c151..a38afacc21 100644 --- a/client/src/network/__tests__/protocol.test.ts +++ b/client/src/network/__tests__/protocol.test.ts @@ -37,7 +37,7 @@ const viewerInteractionWithProducedMana = { describe("encodeWireMessage / decodeWireMessage", () => { it("pins the P2P wire protocol to v18", () => { - expect(WIRE_PROTOCOL_VERSION).toBe(18); + expect(WIRE_PROTOCOL_VERSION).toBe(19); }); it("defaults shortcut actions for a legacy payload created before the additive field", () => { @@ -90,6 +90,7 @@ describe("encodeWireMessage / decodeWireMessage", () => { { type: "reconnect", playerToken: "token-123" }, { type: "reconnect_rejected", reason: "Unknown token" }, { type: "action_rejected", reason: "Player kicked" }, + { type: "action_noop" }, { type: "mana_payment_preview", requestId: 4, sourceIds: [12] }, { type: "mana_payment_preview_rejected", requestId: 4, reason: "Not your turn" }, { diff --git a/client/src/network/protocol.ts b/client/src/network/protocol.ts index 74c920187d..bd37dbf74f 100644 --- a/client/src/network/protocol.ts +++ b/client/src/network/protocol.ts @@ -80,6 +80,10 @@ export function legalActionsFromWire(wire: LegalActionsWire): LegalActionsResult * of silently corrupting state. * * Bumps to date: + * 19 — Added an action_noop acknowledgement for accepted transport no-ops. + * 18 — DebugCardEntries added a serialized, private resolution frame for + * multi-card sandbox battlefield entries that pause for replacement or + * as-enters choices. Old peers cannot deserialize that GameState shape. * 16 — PayableResource::ManaGeneric changed from { per_x } to * { base_cost: ManaCost } (#6410) — a GameState payload field type * change, and base_cost intentionally carries no serde default (a @@ -100,9 +104,6 @@ export function legalActionsFromWire(wire: LegalActionsWire): LegalActionsResult * 9 — Meld pair and attacking-entry choices after mana-payment preview variants. * 8 — Mana-payment preview request/response variants. * 7 — PrecastCopyShortcut action and its two WaitingFor variants. - * 18 — DebugCardEntries added a serialized, private resolution frame for - * multi-card sandbox battlefield entries that pause for replacement or - * as-enters choices. Old peers cannot deserialize that GameState shape. * 17 — Bound draft-match concession request. A Traditional-draft guest * asks its match authority to settle the match; it must not send a * game-level concession through the ordinary P2P path. @@ -110,7 +111,7 @@ export function legalActionsFromWire(wire: LegalActionsWire): LegalActionsResult * sub-phase on WaitingFor::MulliganDecision; the MulliganBottomCards * variant was removed */ -export const WIRE_PROTOCOL_VERSION = 18 as const; +export const WIRE_PROTOCOL_VERSION = 19 as const; export type P2PMessage = P2PAuthorityWire & ( | { type: "guest_deck"; deckData: unknown; displayName?: string; reservationToken?: string } @@ -135,6 +136,7 @@ export type P2PMessage = P2PAuthorityWire & ( logEntries?: GameLogEntry[]; } & LegalActionsWire) | { type: "action_rejected"; reason: string } + | { type: "action_noop" } | { type: "mana_payment_preview"; requestId: number; sourceIds: ObjectId[] } | { type: "mana_payment_preview_rejected"; requestId: number; reason: string } | { type: "ping"; timestamp: number } @@ -196,6 +198,7 @@ const VALID_TYPES = new Set([ "preview_mana_payment", "state_update", "action_rejected", + "action_noop", "mana_payment_preview", "mana_payment_preview_rejected", "ping", diff --git a/crates/engine-wasm/src/lib.rs b/crates/engine-wasm/src/lib.rs index d46694ce0d..d63eab2d97 100644 --- a/crates/engine-wasm/src/lib.rs +++ b/crates/engine-wasm/src/lib.rs @@ -1318,6 +1318,23 @@ pub fn submit_action(actor: u8, action: JsValue) -> JsValue { } } + if let GameAction::Debug(debug_action) = &action { + if debug_action.is_zero_count_create() { + return match with_state(|state| { + engine::game::preflight_debug_action(state, actor, debug_action)?; + Ok::<_, engine::game::EngineError>(engine::types::game_state::ActionResult { + events: vec![], + waiting_for: state.waiting_for.clone(), + log_entries: vec![], + }) + }) { + Ok(Ok(result)) => to_js(&result), + Ok(Err(error)) => JsValue::from_str(&format!("Engine error: {error}")), + Err(error) => error, + }; + } + } + if let GameAction::Debug(engine::types::actions::DebugAction::CreateCard { ref card_name, owner, @@ -1345,20 +1362,10 @@ pub fn submit_action(actor: u8, action: JsValue) -> JsValue { // reaches here. let action_for_replay = action.clone(); let is_debug_action = matches!(action, GameAction::Debug(_)); - let is_zero_count_debug_create = matches!( - &action, - GameAction::Debug(debug_action) if debug_action.is_zero_count_create() - ); match with_state_mut(|state| match apply(state, actor, action) { Ok(result) => { - record_replay_action( - is_debug_action && !is_zero_count_debug_create, - actor, - action_for_replay, - ); - if !is_zero_count_debug_create { - invalidate_ai_proposals(); - } + record_replay_action(is_debug_action, actor, action_for_replay); + invalidate_ai_proposals(); to_js(&result) } Err(e) => { @@ -1399,8 +1406,8 @@ pub fn submit_interaction_js(actor: u8, submission: JsValue) -> JsValue { /// Record a successfully-applied action into REPLAY_LOG, or invalidate any /// in-progress recording if it was a (non-CreateCard) debug action. /// -/// Every `GameAction::Debug` variant other than `CreateCard` reaches this -/// point (unlike CreateCard, they mutate state already tracked by +/// Every successful nonzero `GameAction::Debug` variant other than +/// `CreateCard` reaches this point (unlike CreateCard, they mutate state already tracked by /// `GameState` rather than resolving against the WASM-local `CardDatabase`, /// so they aren't intercepted earlier in `submit_action`) — but /// `reconstruct_initial_state` (`game/replay.rs`) never sets `debug_mode` @@ -1442,7 +1449,7 @@ struct DebugCreateCardRequest<'a> { fn handle_debug_create_card(request: DebugCreateCardRequest<'_>) -> JsValue { match handle_debug_create_card_inner(request) { Ok(result) => to_js(&result), - Err(msg) => JsValue::from_str(msg), + Err(msg) => JsValue::from_str(&msg), } } @@ -1453,7 +1460,7 @@ fn handle_debug_create_card(request: DebugCreateCardRequest<'_>) -> JsValue { /// for the same split. fn handle_debug_create_card_inner( request: DebugCreateCardRequest<'_>, -) -> Result { +) -> Result { let DebugCreateCardRequest { actor, card_name, @@ -1464,59 +1471,43 @@ fn handle_debug_create_card_inner( run_etb, nonlegendary, } = request; - if count > engine::types::actions::MAX_DEBUG_CREATE_COUNT { - return Err("Engine error: debug create count exceeds the maximum"); - } + let debug_action = engine::types::actions::DebugAction::CreateCard { + card_name: card_name.to_string(), + owner, + zone, + count, + attach_to, + run_etb, + nonlegendary, + }; + let waiting_for = with_state(|state| { + engine::game::preflight_debug_action(state, actor, &debug_action) + .map_err(|error| format!("Engine error: {error}"))?; + Ok(state.waiting_for.clone()) + }) + .unwrap_or_else(|_| Err(NOT_INITIALIZED_ERR.to_string()))?; if count == 0 { - return with_state(|state| { - if !state.debug_mode { - return Err("Engine error: Debug actions require debug_mode to be enabled"); - } - if !state.debug_permitted.is_empty() && !state.debug_permitted.contains(&actor) { - return Err("Engine error: Debug actions require debug permission"); - } - if !state.players.iter().any(|player| player.id == owner) { - return Err("Engine error: Debug: invalid owner player id"); - } - Ok(engine::types::game_state::ActionResult { - events: vec![], - waiting_for: state.waiting_for.clone(), - log_entries: vec![], - }) - }) - .unwrap_or(Err(NOT_INITIALIZED_ERR)); + return Ok(engine::types::game_state::ActionResult { + events: vec![], + waiting_for, + log_entries: vec![], + }); } let source = CARD_DB.with(|cell| { let db = cell.borrow(); let Some(db) = db.as_ref() else { - return Err("Engine error: card database not loaded"); + return Err("Engine error: card database not loaded".to_string()); }; match db.get_face_by_name(card_name) { Some(face) => Ok(engine::game::debug_card_entry_source(db, face)), - None => Err("Engine error: card not found in database"), + None => Err("Engine error: card not found in database".to_string()), } })?; with_state_mut(|state| { - if !state.debug_mode { - return Err("Engine error: Debug actions require debug_mode to be enabled"); - } - if !state.debug_permitted.is_empty() && !state.debug_permitted.contains(&actor) { - return Err("Engine error: Debug actions require debug permission"); - } - if !state.players.iter().any(|p| p.id == owner) { - return Err("Engine error: Debug: invalid owner player id"); - } - // Debug-spawned cards are resolved against the WASM-local CARD_DB and - // never recorded into REPLAY_LOG (unlike normal actions in - // `submit_action`), so a faithful replay can't reconstruct this - // mutation. Invalidate any in-progress recording here, the same way - // `restore_game_state` invalidates on a history-breaking state swap, - // so `export_replay_log` can't produce a log that silently omits a - // debug spawn. - REPLAY_LOG.with(|cell| cell.set(None)); let result = engine::game::create_debug_cards( state, engine::game::DebugCardCreateRequest { + actor, source, owner, zone, @@ -1525,14 +1516,23 @@ fn handle_debug_create_card_inner( run_etb, nonlegendary, }, - ); + ) + .map_err(|error| format!("Engine error: {error}"))?; + // Debug-spawned cards are resolved against the WASM-local CARD_DB and + // never recorded into REPLAY_LOG (unlike normal actions in + // `submit_action`), so a faithful replay can't reconstruct this + // mutation. Invalidate any in-progress recording here, the same way + // `restore_game_state` invalidates on a history-breaking state swap, + // so `export_replay_log` can't produce a log that silently omits a + // debug spawn. + REPLAY_LOG.with(|cell| cell.set(None)); engine::game::public_state::bump_state_revision(state); engine::game::public_state::mark_public_state_all_dirty(state); engine::game::public_state::finalize_public_state(state); Ok(result) }) - .unwrap_or(Err(NOT_INITIALIZED_ERR)) + .unwrap_or_else(|_| Err(NOT_INITIALIZED_ERR.to_string())) } /// Get the current game state as a `ClientGameState` wire envelope @@ -4576,10 +4576,24 @@ mod replay_bridge_tests { attach_to: None, run_etb: true, nonlegendary: true, - }); - assert!( - result.is_ok(), - "debug create-card should succeed in this fixture: {result:?}" + }) + .expect("debug create-card should succeed in this fixture"); + assert_eq!( + result + .events + .iter() + .filter(|event| matches!( + event, + engine::types::events::GameEvent::DebugActionUsed { .. } + )) + .count(), + 1, + "the engine source-bound creator owns the audit event" + ); + assert_eq!( + result.log_entries.len(), + 1, + "the engine source-bound creator resolves the local audit log entry" ); with_state(|state| { assert_eq!( @@ -4666,6 +4680,18 @@ mod replay_bridge_tests { result.waiting_for, engine::types::game_state::WaitingFor::Priority { .. } )); + assert_eq!( + result + .events + .iter() + .filter(|event| matches!( + event, + engine::types::events::GameEvent::DebugActionUsed { .. } + )) + .count(), + 1 + ); + assert_eq!(result.log_entries.len(), 1); with_state(|state| { assert_eq!( state @@ -4689,8 +4715,10 @@ mod replay_bridge_tests { #[test] fn debug_create_card_zero_preserves_replay_recording_without_card_database() { clear_game_state(); + CARD_DB.with(|cell| *cell.borrow_mut() = None); let mut state = GameState::new_two_player(17); state.debug_mode = true; + let revision = state.state_revision; REPLAY_LOG.with(|cell| { cell.set(Some(ReplayLog::new(ReplayHeader { format_config: state.format_config.clone(), @@ -4712,10 +4740,88 @@ mod replay_bridge_tests { attach_to: None, run_etb: true, nonlegendary: false, - }); - assert!(result.is_ok()); + }) + .expect("an authorized zero request is a no-op without a card database"); + assert!(result.events.is_empty()); assert!(has_replay_recording()); + with_state(|state| { + assert_eq!(state.state_revision, revision); + assert!(state.objects.is_empty()); + }) + .expect("game state should remain initialized"); + + clear_game_state(); + } + + #[test] + fn debug_create_card_preflight_runs_before_card_database_lookup() { + clear_game_state(); + CARD_DB.with(|cell| *cell.borrow_mut() = None); + let mut state = GameState::new_two_player(23); + state.debug_mode = true; + state.waiting_for = WaitingFor::GameOver { winner: None }; + let revision = state.state_revision; + let public_state_dirty = state.public_state_dirty.clone(); + REPLAY_LOG.with(|cell| { + cell.set(Some(ReplayLog::new(ReplayHeader { + format_config: state.format_config.clone(), + match_config: state.match_config, + player_count: state.players.len() as u8, + first_player: Some(0), + seed: state.rng_seed, + deck_data: None, + }))) + }); + GAME_STATE.with(|cell| cell.set(Some(state))); + + let owner_error = handle_debug_create_card_inner(DebugCreateCardRequest { + actor: PlayerId(0), + card_name: "not loaded", + owner: PlayerId(9), + zone: engine::types::zones::Zone::Hand, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }) + .expect_err("an invalid owner must fail before database access"); + assert!(owner_error.contains("invalid owner player id")); + assert!(!owner_error.contains("database")); + + let priority_error = handle_debug_create_card_inner(DebugCreateCardRequest { + actor: PlayerId(0), + card_name: "not loaded", + owner: PlayerId(0), + zone: engine::types::zones::Zone::Battlefield, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }) + .expect_err("a real entry off Priority must fail before database access"); + assert!(priority_error.contains("Priority window")); + assert!(!priority_error.contains("database")); + let lookup_error = handle_debug_create_card_inner(DebugCreateCardRequest { + actor: PlayerId(0), + card_name: "not loaded", + owner: PlayerId(0), + zone: engine::types::zones::Zone::Hand, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }) + .expect_err("a missing database must reject a valid nonzero request"); + assert!(lookup_error.contains("card database not loaded")); + + assert!(has_replay_recording()); + with_state(|state| { + assert_eq!(state.state_revision, revision); + assert_eq!(state.public_state_dirty, public_state_dirty); + assert!(state.objects.is_empty()); + }) + .expect("game state should remain initialized"); clear_game_state(); } diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 93ba87bcbf..70f28de621 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -6,7 +6,7 @@ use crate::types::ability::{EffectKind, KeywordAction, TargetRef}; #[cfg(test)] use crate::types::ability::{EffectScope, TapStateChange}; use crate::types::actions::{ - GameAction, MayTriggerAutoChoiceOp, PriorityYieldOp, TriggerOrderTemplateOp, + DebugAction, GameAction, MayTriggerAutoChoiceOp, PriorityYieldOp, TriggerOrderTemplateOp, }; use crate::types::events::{BendingType, ContestRound, GameEvent, ManaTapState, PlayerActionKind}; use crate::types::game_state::{ @@ -986,7 +986,7 @@ pub(super) fn apply_action_boundary_with_stack_limit( if let GameAction::Debug(debug_action) = &action { if debug_action.is_zero_count_create() { check_actor_authorization(state, authenticated_actor, &action)?; - check_debug_action_access(state, semantic_owner)?; + preflight_debug_action(state, semantic_owner, debug_action)?; return Ok(ActionResult { events: vec![], waiting_for: state.waiting_for.clone(), @@ -7025,10 +7025,7 @@ fn apply_action( // a defense-in-depth invariant — a player not in `debug_permitted` should // never have reached `apply`. if let GameAction::Debug(debug_action) = action { - check_debug_action_access(state, actor)?; - debug_action - .validate_create_count() - .map_err(EngineError::InvalidAction)?; + preflight_debug_action(state, actor, &debug_action)?; let description = debug_action.describe(state); let mut result = super::engine_debug::apply_debug_action(state, actor, debug_action, &mut events)?; @@ -11578,9 +11575,52 @@ fn apply_action( }) } -/// Sandbox capability check shared by normal debug actions and a zero-count -/// create no-op. Keeping it at the engine boundary means transports cannot use -/// a no-op payload to probe or bypass debug authorization. +/// Validate one debug action before any transport-specific lookup or engine +/// mutation. Source-bound Create Card requests use this same authority again +/// immediately before materialization. +pub fn preflight_debug_action( + state: &GameState, + actor: PlayerId, + action: &DebugAction, +) -> Result<(), EngineError> { + check_debug_action_access(state, actor)?; + action + .validate_create_count() + .map_err(EngineError::InvalidAction)?; + + if let DebugAction::CreateCard { + owner, + zone, + count, + run_etb, + .. + } = action + { + if !state.players.iter().any(|player| player.id == *owner) { + return Err(EngineError::InvalidAction( + "Debug: invalid owner player id".into(), + )); + } + // Real entry can park a private parent frame while replacements or + // as-enters choices resolve. Only a settled Priority boundary can own + // that frame; synchronous hand/raw placements need no such boundary. + if *count != 0 + && *zone == Zone::Battlefield + && *run_etb + && !matches!(state.waiting_for, WaitingFor::Priority { .. }) + { + return Err(EngineError::InvalidAction( + "Debug::CreateCard with ETB processing requires a Priority window".into(), + )); + } + } + + Ok(()) +} + +/// Sandbox capability check shared by every debug-action preflight. Keeping it +/// at the engine boundary means transports cannot use a no-op payload to probe +/// or bypass debug authorization. fn check_debug_action_access(state: &GameState, actor: PlayerId) -> Result<(), EngineError> { if !state.debug_mode { return Err(EngineError::InvalidAction( diff --git a/crates/engine/src/game/engine_debug.rs b/crates/engine/src/game/engine_debug.rs index aed160c845..194972a971 100644 --- a/crates/engine/src/game/engine_debug.rs +++ b/crates/engine/src/game/engine_debug.rs @@ -20,7 +20,7 @@ use crate::types::zones::Zone; use super::effects::attach::{attach_to as attach_object_to, attach_to_player}; use super::effects::change_zone::shuffle_library; -use super::engine::EngineError; +use super::engine::{preflight_debug_action, EngineError}; use super::game_object::AttachTarget; use super::zones; use crate::database::CardDatabase; @@ -32,9 +32,6 @@ pub fn apply_debug_action( action: DebugAction, events: &mut Vec, ) -> Result { - action - .validate_create_count() - .map_err(EngineError::InvalidAction)?; match action { DebugAction::MoveToZone { object_id, @@ -845,6 +842,7 @@ pub fn debug_card_entry_source(db: &CardDatabase, face: &CardFace) -> DebugCardE /// resolved its requested name into a face-complete private source. #[derive(Debug, Clone)] pub struct DebugCardCreateRequest { + pub actor: PlayerId, pub source: DebugCardEntrySource, pub owner: PlayerId, pub zone: Zone, @@ -858,8 +856,31 @@ pub struct DebugCardCreateRequest { /// battlefield creation and explicitly raw battlefield placement complete /// synchronously. Real battlefield entries drain serially through the private /// resolution frame below. -pub fn create_debug_cards(state: &mut GameState, request: DebugCardCreateRequest) -> ActionResult { +pub fn create_debug_cards( + state: &mut GameState, + request: DebugCardCreateRequest, +) -> Result { + let debug_action = DebugAction::CreateCard { + card_name: request.source.face.name.clone(), + owner: request.owner, + zone: request.zone, + count: request.count, + attach_to: request.attach_to, + run_etb: request.run_etb, + nonlegendary: request.nonlegendary, + }; + preflight_debug_action(state, request.actor, &debug_action)?; + if request.count == 0 { + return Ok(ActionResult { + events: vec![], + waiting_for: state.waiting_for.clone(), + log_entries: vec![], + }); + } + let description = debug_action.describe(state); + let before = state.clone(); let DebugCardCreateRequest { + actor, source, owner, zone, @@ -869,15 +890,8 @@ pub fn create_debug_cards(state: &mut GameState, request: DebugCardCreateRequest nonlegendary, } = request; let mut events = Vec::new(); - if count == 0 { - return ActionResult { - events, - waiting_for: state.waiting_for.clone(), - log_entries: vec![], - }; - } - if zone != Zone::Battlefield || !run_etb { + let mut result = if zone != Zone::Battlefield || !run_etb { for _ in 0..count { let initial_zone = if zone == Zone::Battlefield { Zone::Hand @@ -901,34 +915,45 @@ pub fn create_debug_cards(state: &mut GameState, request: DebugCardCreateRequest events.extend(entry.events); } } - return ActionResult { + ActionResult { events, waiting_for: state.waiting_for.clone(), log_entries: vec![], - }; - } - - drain_debug_card_entries( - state, - PendingDebugCardEntries { - source, - owner, - attach_to, - nonlegendary, - remaining: count, - }, - &mut events, - ); - ActionResult { - events, - waiting_for: state.waiting_for.clone(), - log_entries: vec![], - } + } + } else { + drain_debug_card_entries( + state, + PendingDebugCardEntries { + source, + owner, + attach_to, + nonlegendary, + remaining: count, + }, + &mut events, + ); + ActionResult { + events, + waiting_for: state.waiting_for.clone(), + log_entries: vec![], + } + }; + result.events.push(GameEvent::DebugActionUsed { + player_id: actor, + description, + }); + result.log_entries = super::log::resolve_log_entries(&result.events, &before, state); + Ok(result) } /// Resume the active real-entry debug batch after its exact replacement or /// as-enters child has completed. pub(crate) fn drain_pending_debug_card_entries(state: &mut GameState, events: &mut Vec) { + // A non-Priority state belongs to the entry's still-active child. Leave + // the parent frame structurally intact until that child has settled. + if !matches!(state.waiting_for, WaitingFor::Priority { .. }) { + return; + } let Some(pending) = state .take_active_debug_card_entries() .expect("debug-card resumer may consume only its active frame") @@ -1083,6 +1108,151 @@ mod tests { state } + #[test] + fn debug_create_card_preflight_validates_owner_and_real_entry_context() { + let mut state = sandbox_state(); + state.waiting_for = WaitingFor::GameOver { winner: None }; + + let invalid_owner = DebugAction::CreateCard { + card_name: "Debug Creature".into(), + owner: PlayerId(9), + zone: Zone::Hand, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }; + let owner_error = preflight_debug_action(&state, PlayerId(0), &invalid_owner) + .expect_err("CreateCard must name an existing owner"); + assert!(owner_error.to_string().contains("invalid owner player id")); + + let real_entry = DebugAction::CreateCard { + card_name: "Debug Creature".into(), + owner: PlayerId(0), + zone: Zone::Battlefield, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }; + let priority_error = preflight_debug_action(&state, PlayerId(0), &real_entry) + .expect_err("a real battlefield entry may start only from Priority"); + assert!(priority_error.to_string().contains("Priority window")); + + let zero_entry = DebugAction::CreateCard { + card_name: "Debug Creature".into(), + owner: PlayerId(0), + zone: Zone::Battlefield, + count: 0, + attach_to: None, + run_etb: true, + nonlegendary: false, + }; + preflight_debug_action(&state, PlayerId(0), &zero_entry) + .expect("zero is a no-op even off Priority"); + let hand_create = DebugAction::CreateCard { + card_name: "Debug Creature".into(), + owner: PlayerId(0), + zone: Zone::Hand, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }; + preflight_debug_action(&state, PlayerId(0), &hand_create) + .expect("off-battlefield creation is synchronous off Priority"); + let raw_battlefield_create = DebugAction::CreateCard { + card_name: "Debug Creature".into(), + owner: PlayerId(0), + zone: Zone::Battlefield, + count: 1, + attach_to: None, + run_etb: false, + nonlegendary: false, + }; + preflight_debug_action(&state, PlayerId(0), &raw_battlefield_create) + .expect("raw battlefield creation is synchronous off Priority"); + } + + #[test] + fn source_bound_debug_create_preflight_fails_before_materialization() { + let mut state = sandbox_state(); + let revision = state.state_revision; + let error = create_debug_cards( + &mut state, + DebugCardCreateRequest { + actor: PlayerId(0), + source: DebugCardEntrySource { + face: CardFace { + name: "Unmaterialized Debug Card".into(), + ..Default::default() + }, + back_face: None, + }, + owner: PlayerId(9), + zone: Zone::Hand, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }, + ) + .expect_err("the source-bound creator must reuse the shared owner preflight"); + + assert!(error.to_string().contains("invalid owner player id")); + assert!(state.objects.is_empty()); + assert_eq!(state.state_revision, revision); + + state.debug_permitted.insert(PlayerId(0)); + let permission_error = create_debug_cards( + &mut state, + DebugCardCreateRequest { + actor: PlayerId(1), + source: DebugCardEntrySource { + face: CardFace { + name: "Unauthorized Debug Card".into(), + ..Default::default() + }, + back_face: None, + }, + owner: PlayerId(0), + zone: Zone::Hand, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }, + ) + .expect_err("the actor carried by the source-bound request must be authorized"); + assert!(permission_error.to_string().contains("debug permission")); + assert!(state.objects.is_empty()); + assert_eq!(state.state_revision, revision); + } + + #[test] + fn zero_debug_create_card_uses_the_shared_owner_preflight() { + let mut state = sandbox_state(); + let revision = state.state_revision; + let error = crate::game::engine::apply( + &mut state, + PlayerId(0), + GameAction::Debug(DebugAction::CreateCard { + card_name: "No Card Needed".into(), + owner: PlayerId(9), + zone: Zone::Battlefield, + count: 0, + attach_to: None, + run_etb: true, + nonlegendary: false, + }), + ) + .expect_err("the action-boundary zero fast path must validate CreateCard owner"); + + assert!(error.to_string().contains("invalid owner player id")); + assert_eq!(state.state_revision, revision); + assert!(state.objects.is_empty()); + } + #[test] fn debug_create_card_batch_enters_battlefield_serially() { let mut state = sandbox_state(); @@ -1097,6 +1267,7 @@ mod tests { let result = create_debug_cards( &mut state, DebugCardCreateRequest { + actor: PlayerId(0), source, owner: PlayerId(0), zone: Zone::Battlefield, @@ -1105,9 +1276,11 @@ mod tests { run_etb: true, nonlegendary: false, }, - ); + ) + .expect("an authorized debug batch should succeed"); assert!(matches!(result.waiting_for, WaitingFor::Priority { .. })); + assert_eq!(result.log_entries.len(), 1); assert!(state.resolution_stack.is_empty()); assert_eq!( state @@ -1180,6 +1353,7 @@ mod tests { let result = create_debug_cards( &mut state, DebugCardCreateRequest { + actor: PlayerId(0), source: DebugCardEntrySource { face: CardFace { name: "Paused Debug Batch Creature".into(), @@ -1194,12 +1368,27 @@ mod tests { run_etb: true, nonlegendary: false, }, - ); + ) + .expect("an authorized debug batch should start"); assert!(matches!( result.waiting_for, WaitingFor::ReplacementChoice { .. } )); + assert_eq!( + result + .events + .iter() + .filter(|event| matches!(event, GameEvent::DebugActionUsed { .. })) + .count(), + 1, + "the source-bound action emits one audit event when the batch starts" + ); + assert_eq!( + result.log_entries.len(), + 1, + "the source-bound engine path resolves its audit log entry" + ); assert_eq!( state .objects @@ -1222,21 +1411,43 @@ mod tests { .is_empty(), "the private source/frame never crosses a viewer-state boundary" ); + let pending_before = state + .active_debug_card_entries() + .cloned() + .expect("the remaining batch member is active"); + let mut premature_events = Vec::new(); + drain_pending_debug_card_entries(&mut state, &mut premature_events); + assert_eq!( + state.active_debug_card_entries(), + Some(&pending_before), + "an off-Priority resume attempt must not consume the batch frame" + ); + assert!(premature_events.is_empty()); let persisted = PersistedGameState::capture(state); let serialized = serde_json::to_string(&persisted).expect("paused batch serializes"); let persisted: PersistedGameState = serde_json::from_str(&serialized).expect("paused batch deserializes"); let mut restored = persisted.into_game_state(); - apply_as_current(&mut restored, GameAction::ChooseReplacement { index: 0 }) - .expect("replacement choice resumes the serial batch"); + let first_resume = + apply_as_current(&mut restored, GameAction::ChooseReplacement { index: 0 }) + .expect("replacement choice resumes the serial batch"); + assert!(first_resume + .events + .iter() + .all(|event| !matches!(event, GameEvent::DebugActionUsed { .. }))); assert!(matches!( restored.waiting_for, WaitingFor::ReplacementChoice { .. } )); - apply_as_current(&mut restored, GameAction::ChooseReplacement { index: 0 }) - .expect("the remaining entrant presents and resumes its own replacement choice"); + let second_resume = + apply_as_current(&mut restored, GameAction::ChooseReplacement { index: 0 }) + .expect("the remaining entrant presents and resumes its own replacement choice"); + assert!(second_resume + .events + .iter() + .all(|event| !matches!(event, GameEvent::DebugActionUsed { .. }))); assert!(matches!(restored.waiting_for, WaitingFor::Priority { .. })); assert!(restored.resolution_stack.is_empty()); diff --git a/crates/engine/src/game/mod.rs b/crates/engine/src/game/mod.rs index f656c910bf..f4df229496 100644 --- a/crates/engine/src/game/mod.rs +++ b/crates/engine/src/game/mod.rs @@ -217,8 +217,8 @@ pub use deck_validation::{ DeckCompatibilityResult, DeckCoverage, SignatureSpellSelectionPolicy, UnsupportedCard, }; pub use engine::{ - apply, apply_as_current, new_game, start_game, start_game_skip_mulligan, - start_game_with_starting_player, EngineError, + apply, apply_as_current, new_game, preflight_debug_action, start_game, + start_game_skip_mulligan, start_game_with_starting_player, EngineError, }; pub use engine_debug::{ create_debug_cards, debug_card_entry_source, route_debug_create_to_battlefield, diff --git a/crates/lobby-broker/src/protocol.rs b/crates/lobby-broker/src/protocol.rs index bddf10a7d5..d70fc80709 100644 --- a/crates/lobby-broker/src/protocol.rs +++ b/crates/lobby-broker/src/protocol.rs @@ -43,6 +43,7 @@ pub enum ServerErrorCode { /// handshake. When making such changes, plan a deprecation window where /// both the old and new variants coexist, then bump and remove the old. /// +/// 26 — Added `ServerMessage::ActionNoOp` for accepted transport no-ops. /// 25 — `DebugCardEntries` added a serialized, private resolution frame for /// multi-card sandbox battlefield entries that pause for replacement or /// as-enters choices. Old peers cannot deserialize that `GameState` shape. @@ -72,7 +73,7 @@ pub enum ServerErrorCode { /// payload; mulligan bottoming folded into a /// `MulliganDecisionPhase::BottomCards` sub-phase on /// `WaitingFor::MulliganDecision`. -pub const PROTOCOL_VERSION: u32 = 25; +pub const PROTOCOL_VERSION: u32 = 26; /// Minimum protocol version accepted by lobby-only brokers at the hello /// handshake. Lobby traffic has a one-version rollout window; full game servers @@ -408,13 +409,13 @@ mod tests { use super::*; #[test] - fn protocol_version_tracks_priority_passing_wire_additions() { - assert_eq!(PROTOCOL_VERSION, 25); + fn protocol_version_tracks_full_game_wire_additions() { + assert_eq!(PROTOCOL_VERSION, 26); // Lobby keeps its one-version rollout window; full-game servers stay // current-only (`server_core::MIN_SUPPORTED_PROTOCOL == PROTOCOL_VERSION`), // which is what refuses an older full-game peer whose GameState cannot - // carry a paused DebugCardEntries resolution frame. - assert_eq!(MIN_SUPPORTED_PROTOCOL, 24); + // understand a success acknowledgment the submitting client awaits. + assert_eq!(MIN_SUPPORTED_PROTOCOL, 25); } #[test] diff --git a/crates/phase-server/src/main.rs b/crates/phase-server/src/main.rs index f6d75af111..c7851761c1 100644 --- a/crates/phase-server/src/main.rs +++ b/crates/phase-server/src/main.rs @@ -3763,6 +3763,17 @@ impl GameSubmission { } } + /// Accepted zero-count debug creates are transport no-ops: server-core + /// still authenticates and preflights them, but the Full-mode wrapper must + /// not allocate a revision, run AI, persist, or broadcast unchanged state. + fn is_zero_count_debug_create(&self) -> bool { + matches!( + self, + GameSubmission::Action(GameAction::Debug(debug_action)) + if debug_action.is_zero_count_create() + ) + } + fn payload_rejection(&self) -> Result<(), Box> { match self { GameSubmission::Action(action) => guard_game_action_payload(action) @@ -3800,6 +3811,7 @@ async fn handle_full_game_submission( identity: &SocketIdentity, ) { let kind = submission.kind(); + let is_zero_count_debug_create = submission.is_zero_count_debug_create(); let game_code = match &identity.game_code { Some(c) => c.clone(), None => { @@ -3850,6 +3862,13 @@ async fn handle_full_game_submission( }; match applied { Ok(human_result) => { + if is_zero_count_debug_create { + drop(mgr); + if let Ok(json) = serde_json::to_string(&ServerMessage::ActionNoOp) { + let _ = socket.send(Message::text(json)).await; + } + return; + } let human_revision = mgr .sessions .get_mut(&game_code) @@ -8236,7 +8255,9 @@ mod game_submission_tests { use super::issue_4548_full_create_tests::{recv_server_message, spawn_full_mode_server}; use super::*; use engine::game::interaction::MAX_INTERACTION_STRING_LEN; + use engine::types::actions::DebugAction; use engine::types::interaction::{InteractionChoiceId, InteractionId, InteractionResponse}; + use engine::types::zones::Zone; use futures_util::SinkExt; use server_core::game_action_payload_guard::MAX_ACTION_LIST_LEN; use server_core::protocol::DeckData; @@ -8244,6 +8265,25 @@ mod game_submission_tests { use tokio_tungstenite::MaybeTlsStream; use tokio_tungstenite::WebSocketStream; + #[test] + fn zero_count_debug_create_is_the_only_submission_no_op() { + let create_card = |count| { + GameSubmission::Action(GameAction::Debug(DebugAction::CreateCard { + card_name: "Lightning Bolt".to_string(), + owner: PlayerId(0), + zone: Zone::Hand, + count, + attach_to: None, + run_etb: false, + nonlegendary: false, + })) + }; + + assert!(create_card(0).is_zero_count_debug_create()); + assert!(!create_card(1).is_zero_count_debug_create()); + assert!(!GameSubmission::Action(GameAction::PassPriority).is_zero_count_debug_create()); + } + /// Connect, handshake, and create a two-seat game so the socket carries an /// authenticated `SocketIdentity` with both a `game_code` and a /// `player_token`. diff --git a/crates/server-core/src/protocol.rs b/crates/server-core/src/protocol.rs index 2106f6cf41..9d5b97500b 100644 --- a/crates/server-core/src/protocol.rs +++ b/crates/server-core/src/protocol.rs @@ -494,6 +494,10 @@ pub enum ServerMessage { ActionRejected { reason: String, }, + /// Confirms an authenticated action that intentionally produced no state + /// transition. The submitting adapter resolves its pending request without + /// caching or publishing a replacement snapshot. + ActionNoOp, /// Acknowledges a host-authorized permanent game cleanup. GameAbandoned { game_code: String, @@ -2257,18 +2261,18 @@ mod tests { } #[test] - fn protocol_version_is_25() { - assert_eq!(PROTOCOL_VERSION, 25); + fn protocol_version_is_26() { + assert_eq!(PROTOCOL_VERSION, 26); } /// The bump alone is inert — a version number nobody enforces prevents no /// pairing. This is the assertion with teeth: full-game servers accept ONLY /// the current protocol, so an older peer cannot complete a handshake with - /// a server that may persist a paused DebugCardEntries frame it cannot - /// deserialize. + /// a server that may answer an accepted action on a variant it does not + /// understand. /// /// REVERT-PROBE: relax to `PROTOCOL_VERSION - 1` — the exact regression - /// this guards — and this test reds while `protocol_version_is_24` stays + /// this guards — and this test reds while `protocol_version_is_26` stays /// green, which is why the two are separate assertions. #[test] fn full_game_floor_is_current_only_not_a_rollout_window() { @@ -2420,6 +2424,16 @@ mod tests { assert!(matches!(parsed, ClientMessage::CancelTakeback)); } + #[test] + fn server_message_action_no_op_roundtrips() { + let json = serde_json::to_string(&ServerMessage::ActionNoOp).unwrap(); + assert_eq!(json, r#"{"type":"ActionNoOp"}"#); + assert!(matches!( + serde_json::from_str::(&json).unwrap(), + ServerMessage::ActionNoOp + )); + } + #[test] fn server_message_takeback_requested_roundtrips() { let msg = ServerMessage::TakebackRequested { diff --git a/crates/server-core/src/session.rs b/crates/server-core/src/session.rs index 0f2ee624c5..7c1de933e3 100644 --- a/crates/server-core/src/session.rs +++ b/crates/server-core/src/session.rs @@ -8,14 +8,13 @@ use engine::database::CardDatabase; use engine::game::deck_loading::{DeckPayload, PlayerDeckPayload}; use engine::game::engine::{apply, start_game}; use engine::game::interaction::{bind_interaction_authority, submit_interaction}; -use engine::game::log::resolve_log_entries; use engine::game::match_flow::apply_trusted_match_forfeit; use engine::game::preview::preview_auto_payment_sources; use engine::game::public_state::{ bump_state_revision, finalize_public_state, mark_public_state_all_dirty, }; use engine::game::{ - create_debug_cards, debug_card_entry_source, load_and_hydrate_decks, + create_debug_cards, debug_card_entry_source, load_and_hydrate_decks, preflight_debug_action, rehydrate_game_from_card_db, DebugCardCreateRequest, }; use engine::types::actions::{DebugAction, GameAction}; @@ -1427,12 +1426,39 @@ impl SessionManager { // access; the engine validates actor authorization and action shape. // Candidate enumeration is advisory for clients and AI, not a second // legality gate: several legal action classes are combinatorial. - let records_takeback = !action.is_actor_scoped_preference(); - if let GameAction::Debug(debug_action) = &action { - debug_action.validate_create_count()?; + preflight_debug_action(&session.state, player, debug_action) + .map_err(|error| format!("Engine error: {error}"))?; + } + if matches!(&action, GameAction::Debug(debug_action) if debug_action.is_zero_count_create()) + { + let (legal_actions, spell_costs, by_object) = engine_legal_actions_full(&session.state); + let auto_pass = auto_pass_recommended(&session.state, &legal_actions); + return Ok(( + session.state.clone(), + Vec::new(), + legal_actions, + Vec::new(), + auto_pass, + spell_costs, + by_object, + )); } + let debug_card_source = match &action { + GameAction::Debug(DebugAction::CreateCard { card_name, .. }) => { + let card_db = card_db.ok_or_else(|| { + "Debug::CreateCard requires a card database at the transport boundary" + .to_string() + })?; + let face = card_db + .get_face_by_name(card_name) + .ok_or_else(|| "Engine error: card not found in database".to_string())?; + Some(debug_card_entry_source(card_db, face)) + } + _ => None, + }; + let records_takeback = !action.is_actor_scoped_preference(); let pre_action_state = records_takeback.then(|| session.state.clone()); // Set player names for log resolution. @@ -1445,48 +1471,21 @@ impl SessionManager { // wire is rejected inside the engine as well as here. let action_type = action.variant_name(); let result = match action { - action @ GameAction::Debug(DebugAction::CreateCard { count: 0, .. }) => { - apply(&mut session.state, player, action).map_err(|e| { - warn!(game = %game_code, player = ?player, error = %e, reason = "engine_error", "action rejected"); - format!("Engine error: {}", e) - })? - } GameAction::Debug(DebugAction::CreateCard { - card_name, owner, zone, count, attach_to, run_etb, nonlegendary, + .. }) => { - if !session.state.debug_mode { - return Err("Engine error: Debug actions require debug_mode to be enabled".to_string()); - } - if !session.state.players.iter().any(|player| player.id == owner) { - return Err("Engine error: Debug: invalid owner player id".to_string()); - } - let card_db = card_db.ok_or_else(|| { - "Debug::CreateCard requires a card database at the transport boundary".to_string() - })?; - let face = card_db - .get_face_by_name(&card_name) - .ok_or_else(|| "Engine error: card not found in database".to_string())?; - let debug_action = DebugAction::CreateCard { - card_name: card_name.clone(), - owner, - zone, - count, - attach_to, - run_etb, - nonlegendary, - }; - let description = debug_action.describe(&session.state); - let before = session.state.clone(); - let mut result = create_debug_cards( + let result = create_debug_cards( &mut session.state, DebugCardCreateRequest { - source: debug_card_entry_source(card_db, face), + actor: player, + source: debug_card_source + .expect("nonzero debug CreateCard source was bound before mutation"), owner, zone, count, @@ -1494,15 +1493,11 @@ impl SessionManager { run_etb, nonlegendary, }, - ); - result.events.push(GameEvent::DebugActionUsed { - player_id: player, - description, - }); + ) + .map_err(|error| format!("Engine error: {error}"))?; bump_state_revision(&mut session.state); mark_public_state_all_dirty(&mut session.state); finalize_public_state(&mut session.state); - result.log_entries = resolve_log_entries(&result.events, &before, &session.state); result } action => apply(&mut session.state, player, action).map_err(|e| { @@ -4054,7 +4049,7 @@ mod tests { &token, GameAction::Debug(engine::types::actions::DebugAction::CreateCard { card_name: "Server Debug Creature".into(), - owner: PlayerId(0), + owner: PlayerId(1), zone: Zone::Battlefield, count: 2, attach_to: None, @@ -4065,12 +4060,22 @@ mod tests { ) .expect("server transport resolves a debug CreateCard batch through its card database"); - assert!( + assert_eq!( result .1 .iter() - .any(|event| matches!(event, GameEvent::DebugActionUsed { .. })), - "source-bound server debug actions retain the engine audit event" + .filter(|event| { + matches!( + event, + GameEvent::DebugActionUsed { + player_id: PlayerId(0), + .. + } + ) + }) + .count(), + 1, + "source-bound server debug actions retain exactly the engine audit event" ); assert!( !result.3.is_empty(), @@ -4083,13 +4088,158 @@ mod tests { .objects .values() .filter(|object| { - object.name == "Server Debug Creature" && object.zone == Zone::Battlefield + object.name == "Server Debug Creature" + && object.owner == PlayerId(1) + && object.zone == Zone::Battlefield }) .count(), 2 ); } + #[test] + fn server_debug_create_zeroes_skip_lifecycle_and_takeback_side_effects() { + let mut mgr = SessionManager::new(); + let (code, token) = create_sandbox_game(&mut mgr); + let session = &mgr.sessions[&code]; + let history_depth = session.takeback_history.len(); + let turn_history_depth = session.turn_rewind_history.len(); + let rewind_game_number = session.rewind_game_number; + let session_revision = session.state_revision; + let revision = session.state.state_revision; + let object_count = session.state.objects.len(); + let log_player_names = session.state.log_player_names.clone(); + + let actions = [ + ( + "card", + GameAction::Debug(DebugAction::CreateCard { + card_name: "database deliberately absent".into(), + owner: PlayerId(0), + zone: Zone::Battlefield, + count: 0, + attach_to: None, + run_etb: true, + nonlegendary: false, + }), + ), + ( + "token", + GameAction::Debug(DebugAction::CreateToken { + request: engine::types::actions::DebugTokenRequest::Preset { + preset_id: "not resolved for zero".into(), + owner: PlayerId(0), + power_override: None, + toughness_override: None, + enter_with_counters: Vec::new(), + }, + count: 0, + run_etb: true, + }), + ), + ( + "token copy", + GameAction::Debug(DebugAction::CreateTokenCopy { + source_id: ObjectId(u64::MAX), + owner: PlayerId(0), + count: 0, + nonlegendary: false, + }), + ), + ]; + + for (label, action) in actions { + let result = mgr + .handle_action(&code, &token, action) + .unwrap_or_else(|error| panic!("authorized zero {label} must be a no-op: {error}")); + + assert!(result.1.is_empty(), "zero {label} emitted events"); + assert!(result.3.is_empty(), "zero {label} emitted log entries"); + } + let session = &mgr.sessions[&code]; + assert_eq!(session.takeback_history.len(), history_depth); + assert_eq!(session.turn_rewind_history.len(), turn_history_depth); + assert_eq!(session.rewind_game_number, rewind_game_number); + assert_eq!(session.state_revision, session_revision); + assert_eq!(session.state.state_revision, revision); + assert_eq!(session.state.objects.len(), object_count); + assert_eq!(session.state.log_player_names, log_player_names); + } + + #[test] + fn server_debug_create_preflight_runs_before_database_lookup() { + let mut mgr = SessionManager::new(); + let (code, token) = create_sandbox_game(&mut mgr); + + let owner_error = mgr + .handle_action( + &code, + &token, + GameAction::Debug(DebugAction::CreateCard { + card_name: "database deliberately absent".into(), + owner: PlayerId(9), + zone: Zone::Hand, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }), + ) + .expect_err("an invalid owner must fail before database lookup"); + assert!(owner_error.contains("invalid owner player id")); + assert!(!owner_error.contains("card database")); + + let session = &mgr.sessions[&code]; + let history_depth = session.takeback_history.len(); + let revision = session.state.state_revision; + let public_state_dirty = session.state.public_state_dirty.clone(); + let log_player_names = session.state.log_player_names.clone(); + let lookup_error = mgr + .handle_action( + &code, + &token, + GameAction::Debug(DebugAction::CreateCard { + card_name: "database deliberately absent".into(), + owner: PlayerId(0), + zone: Zone::Hand, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }), + ) + .expect_err("a valid nonzero request requires a database"); + assert!(lookup_error.contains("requires a card database")); + let session = &mgr.sessions[&code]; + assert_eq!(session.takeback_history.len(), history_depth); + assert_eq!(session.state.state_revision, revision); + assert_eq!(session.state.public_state_dirty, public_state_dirty); + assert_eq!(session.state.log_player_names, log_player_names); + + mgr.sessions + .get_mut(&code) + .expect("sandbox session exists") + .state + .waiting_for = WaitingFor::GameOver { winner: None }; + let priority_error = mgr + .handle_action( + &code, + &token, + GameAction::Debug(DebugAction::CreateCard { + card_name: "database deliberately absent".into(), + owner: PlayerId(0), + zone: Zone::Battlefield, + count: 1, + attach_to: None, + run_etb: true, + nonlegendary: false, + }), + ) + .expect_err("a real entry off Priority must fail before database lookup"); + assert!(priority_error.contains("Priority window")); + assert!(!priority_error.contains("card database")); + } + #[test] fn with_sandbox_sets_flag_and_is_idempotent() { let base = FormatConfig::standard(); diff --git a/scripts/check-protocol-version.mjs b/scripts/check-protocol-version.mjs index 524c2db62a..c07854ed4e 100644 --- a/scripts/check-protocol-version.mjs +++ b/scripts/check-protocol-version.mjs @@ -3,7 +3,7 @@ import { dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); -const EXPECTED_PROTOCOL_VERSION = 25; +const EXPECTED_PROTOCOL_VERSION = 26; function extractVersion(source, pattern, label) { const match = source.match(pattern); From 062cd5f7c640bba008cc09f03b60fc130db99b6e Mon Sep 17 00:00:00 2001 From: matthewevans Date: Sun, 9 Aug 2026 03:31:46 -0700 Subject: [PATCH 3/3] test(debug): correct batch entry assertions --- crates/engine-wasm/src/lib.rs | 1 - crates/engine/src/game/engine.rs | 5 ++++- crates/engine/src/game/engine_debug.rs | 1 - 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/engine-wasm/src/lib.rs b/crates/engine-wasm/src/lib.rs index d63eab2d97..1ada5f89b9 100644 --- a/crates/engine-wasm/src/lib.rs +++ b/crates/engine-wasm/src/lib.rs @@ -4691,7 +4691,6 @@ mod replay_bridge_tests { .count(), 1 ); - assert_eq!(result.log_entries.len(), 1); with_state(|state| { assert_eq!( state diff --git a/crates/engine/src/game/engine.rs b/crates/engine/src/game/engine.rs index 70f28de621..cba5c6b881 100644 --- a/crates/engine/src/game/engine.rs +++ b/crates/engine/src/game/engine.rs @@ -16350,9 +16350,12 @@ mod stage2_injector_tests { // knowledge) = 11874`, which equals the located coordinate exactly. The conflict markers // sat BELOW the producer, so resolving them could not have shifted it. // + // #7128 adds forty lines above this producer while introducing the source-bound debug + // card entry boundary. The producer remains byte-identical; only its coordinate moves. + // // SET PRESERVATION: unchanged. Upstream adds no line matching the needle to this file and // neither does this branch — total still 37, partition still 5/7/25. - "game/engine.rs:11902".to_string(), + "game/engine.rs:11942".to_string(), ], "the five production producers, NAMED: the CR 603.5 gate in `resolve_chain_body` \ plus the two repeated-optional-payment drivers, the per-player acceptance cursor \ diff --git a/crates/engine/src/game/engine_debug.rs b/crates/engine/src/game/engine_debug.rs index 194972a971..db293d883c 100644 --- a/crates/engine/src/game/engine_debug.rs +++ b/crates/engine/src/game/engine_debug.rs @@ -1280,7 +1280,6 @@ mod tests { .expect("an authorized debug batch should succeed"); assert!(matches!(result.waiting_for, WaitingFor::Priority { .. })); - assert_eq!(result.log_entries.len(), 1); assert!(state.resolution_stack.is_empty()); assert_eq!( state