Skip to content
Merged
32 changes: 30 additions & 2 deletions client/src/adapter/__tests__/ws-adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,34 @@ describe("WebSocketAdapter", () => {
await expect(resultPromise).rejects.toMatchObject({ message: "batch snapshot rejected" });
});

it("scopes the stale priority race to correlated Resolve All rejections", async () => {
const stale = adapter.resolveAll(0, [{ playerId: 1, difficulty: "Medium" }], 5);
ws.dispatchSynthetic(
"message",
JSON.stringify({
type: "ResolveAllRejected",
data: { request_id: 1, reason: "Resolve All requires your priority" },
}),
);
await expect(stale).rejects.toMatchObject({
code: "STALE_ACTION",
recoverable: false,
});

const rejected = adapter.resolveAll(0, [{ playerId: 1, difficulty: "Medium" }], 5);
ws.dispatchSynthetic(
"message",
JSON.stringify({
type: "ResolveAllRejected",
data: { request_id: 2, reason: "batch snapshot rejected" },
}),
);
await expect(rejected).rejects.toMatchObject({
code: "ACTION_REJECTED",
recoverable: true,
});
});

describe("server rewind capability (F2)", () => {
it("declares the capability through the standalone type guard", () => {
expect(supportsServerRewind(adapter)).toBe(true);
Expand Down Expand Up @@ -997,13 +1025,13 @@ describe("WebSocketAdapter", () => {
});
});

it("still surfaces a non-stale server rejection as a recoverable ACTION_REJECTED", async () => {
it("keeps the Resolve All priority text actionable on an ordinary action rejection", async () => {
const pending = adapter.submitAction({ type: "PassPriority" }, 0);
ws.dispatchSynthetic(
"message",
JSON.stringify({
type: "ActionRejected",
data: { reason: "Engine error: Something genuinely wrong" },
data: { reason: "Resolve All requires your priority" },
}),
);
await expect(pending).rejects.toMatchObject({
Expand Down
15 changes: 15 additions & 0 deletions client/src/adapter/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3568,6 +3568,21 @@ export function actionRejectionError(reason: string): AdapterError {
: new AdapterError(AdapterErrorCode.ACTION_REJECTED, reason, true);
}

/**
* Classify a requester-correlated Resolve All rejection.
*
* A batch request can reach the server after priority has advanced. The server
* must reject that request, but this particular response is a stale UI race,
* not an actionable error for the requester. Keep the classification scoped to
* the Resolve All protocol frame: the same text on an ordinary action
* rejection must remain visible.
*/
export function resolveAllRejectionError(reason: string): AdapterError {
return reason === "Resolve All requires your priority"
? new AdapterError(AdapterErrorCode.STALE_ACTION, reason, false)
: actionRejectionError(reason);
}

/**
* Detect the engine's rejection of a `ReorderHand` whose order no longer names
* the current hand. `apply_action` formats
Expand Down
4 changes: 2 additions & 2 deletions client/src/adapter/ws-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type {
FormatConfig,
} from "./types";
import type { InteractionSubmission } from "./generated/interaction";
import { AdapterError, AdapterErrorCode, EMPTY_LEGAL_ACTIONS, actionRejectionError, nextSnapshotSeq } from "./types";
import { AdapterError, AdapterErrorCode, EMPTY_LEGAL_ACTIONS, actionRejectionError, nextSnapshotSeq, resolveAllRejectionError } from "./types";
import type { BracketDeckRequest, BracketEstimate } from "../types/bracketEstimate";
import {
HandshakeError,
Expand Down Expand Up @@ -1529,7 +1529,7 @@ export class WebSocketAdapter implements EngineAdapter {
case "ResolveAllRejected": {
const data = msg.data as { request_id: number; reason: string };
if (this.pendingResolveAll?.requestId === data.request_id) {
this.pendingResolveAll.reject(actionRejectionError(data.reason));
this.pendingResolveAll.reject(resolveAllRejectionError(data.reason));
this.pendingResolveAll = null;
}
break;
Expand Down
37 changes: 33 additions & 4 deletions client/src/game/__tests__/dispatchResolveAll.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

import type { BatchResolveResult, EngineSnapshot, GameState } from "../../adapter/types";
import { nextSnapshotSeq } from "../../adapter/types";
import { AdapterError, AdapterErrorCode, nextSnapshotSeq } from "../../adapter/types";
import { useGameStore } from "../../stores/gameStore";
import { useAppNotificationStore } from "../../stores/appToastStore";
import { usePreferencesStore } from "../../stores/preferencesStore";
Expand Down Expand Up @@ -191,10 +191,39 @@ describe("dispatchResolveAll progress", () => {
expect(submitAction).not.toHaveBeenCalled();
});

it("shows a server-provided Resolve All rejection without rejecting the click handler", async () => {
it("silently absorbs a stale Resolve All priority rejection without rejecting the click handler", async () => {
const resolveAll = vi
.fn<EngineResolveAll>()
.mockRejectedValue(new Error("Resolve All requires your priority"));
.mockRejectedValue(
new AdapterError(
AdapterErrorCode.STALE_ACTION,
"Resolve All requires your priority",
false,
),
);
const getState = vi.fn().mockResolvedValue(stateWithStack(2));
useGameStore.setState({
gameState: stateWithStack(2),
adapter: {
resolveAll,
resolveAllUsesServerAi: true,
getState,
getLegalActions: vi.fn().mockResolvedValue({ actions: [], autoPassRecommended: false }),
getSnapshot: snapshotVia(getState),
} as never,
});

await expect(dispatchResolveAll(0, [])).resolves.toBeUndefined();

expect(useAppNotificationStore.getState().notification).toBeNull();
expect(useGameStore.getState().isResolvingAll).toBe(false);
expect(useGameStore.getState().resolutionProgress).toBeNull();
});

it("still surfaces a non-stale Resolve All rejection", async () => {
const resolveAll = vi
.fn<EngineResolveAll>()
.mockRejectedValue(new Error("batch snapshot rejected"));
const getState = vi.fn().mockResolvedValue(stateWithStack(2));
useGameStore.setState({
gameState: stateWithStack(2),
Expand All @@ -210,7 +239,7 @@ describe("dispatchResolveAll progress", () => {
await expect(dispatchResolveAll(0, [])).resolves.toBeUndefined();

expect(useAppNotificationStore.getState().notification).toMatchObject({
description: "Resolve All requires your priority",
description: "batch snapshot rejected",
});
});

Expand Down
1 change: 1 addition & 0 deletions client/src/game/dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1080,6 +1080,7 @@ export async function dispatchResolveAll(
await saveAuthoritativeGame(gameId, adapter, newState);
}
} catch (err) {
if (isStaleAction(err)) return;
debugLog(`Resolve All error: ${err instanceof Error ? err.message : String(err)}`);
showActionError({ type: "PassPriority" }, err);
} finally {
Expand Down
26 changes: 6 additions & 20 deletions crates/engine/src/ai_support/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,26 +103,12 @@ impl AiDecisionContract {
}

pub(crate) fn target_selection_requires_reducer_validation(state: &GameState) -> bool {
let WaitingFor::TargetSelection {
player,
pending_cast,
target_slots,
selection,
..
} = &state.waiting_for
else {
return false;
};

// Only the final target can lock a target-dependent cost. Earlier
// selections are valid reducer continuations regardless of whether the
// eventual cost is payable.
selection.current_slot.checked_add(1) == Some(target_slots.len())
&& !crate::game::casting::pending_mana_obligation_is_stable_before_targets(
state,
*player,
pending_cast,
)
// CR 601.2c + CR 601.2e-h + CR 602.2b: selecting a target can complete
// target declaration and immediately check legality and pay the proposed
// spell or activation cost. A later optional slot can become auto-skippable
// only after this target is chosen, so the reducer is the sole authority for
// whether a particular candidate completes the transition.
matches!(&state.waiting_for, WaitingFor::TargetSelection { .. })
}

/// Whether a decision can alter the target requirements of an in-progress cast.
Expand Down
67 changes: 42 additions & 25 deletions crates/engine/src/ai_support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6171,7 +6171,7 @@ mod tests {
}

#[test]
fn target_selection_legal_actions_use_current_targets_without_simulation() {
fn target_selection_legal_actions_use_current_targets_with_reducer_validation() {
let mut state = setup_priority();
let targets: Vec<TargetRef> = (0..25)
.map(|i| {
Expand All @@ -6198,13 +6198,22 @@ mod tests {
state.waiting_for = WaitingFor::TargetSelection {
player: PlayerId(0),
pending_cast,
target_slots: vec![crate::types::game_state::TargetSelectionSlot {
legal_targets: targets.clone(),
optional: true,
chooser: None,
effect_kind: EffectKind::NoOp,
effect_detail: TargetEffectDetail::None,
}],
target_slots: vec![
crate::types::game_state::TargetSelectionSlot {
legal_targets: targets.clone(),
optional: true,
chooser: None,
effect_kind: EffectKind::NoOp,
effect_detail: TargetEffectDetail::None,
},
crate::types::game_state::TargetSelectionSlot {
legal_targets: vec![targets[0].clone()],
optional: true,
chooser: None,
effect_kind: EffectKind::NoOp,
effect_detail: TargetEffectDetail::None,
},
],
Comment thread
coderabbitai[bot] marked this conversation as resolved.
mode_labels: Vec::new(),
selection: crate::types::game_state::TargetSelectionProgress {
current_slot: 0,
Expand All @@ -6213,12 +6222,8 @@ mod tests {
},
};

crate::game::perf_counters::reset();
let (actions, spell_costs, grouped) = legal_actions_full(&state);
let counters = crate::game::perf_counters::snapshot();

assert_eq!(counters.state_clone_for_legality, 0);
assert_eq!(counters.priority_cast_probe_builds, 0);
assert_eq!(
actions
.iter()
Expand All @@ -6227,7 +6232,8 @@ mod tests {
25
);
assert!(actions.contains(&GameAction::ChooseTarget { target: None }));
assert_eq!(actions.len(), 26);
assert_eq!(actions.len(), 27);
assert!(actions.contains(&GameAction::CancelCast));
assert!(spell_costs.is_empty());
assert!(grouped.is_empty());
assert!(actions
Expand Down Expand Up @@ -6601,13 +6607,22 @@ mod tests {
state.waiting_for = WaitingFor::TargetSelection {
player: PlayerId(0),
pending_cast,
target_slots: vec![crate::types::game_state::TargetSelectionSlot {
legal_targets: vec![target.clone()],
optional: true,
chooser: None,
effect_kind: EffectKind::NoOp,
effect_detail: TargetEffectDetail::None,
}],
target_slots: vec![
crate::types::game_state::TargetSelectionSlot {
legal_targets: vec![target.clone()],
optional: true,
chooser: None,
effect_kind: EffectKind::NoOp,
effect_detail: TargetEffectDetail::None,
},
crate::types::game_state::TargetSelectionSlot {
legal_targets: vec![target.clone()],
optional: true,
chooser: None,
effect_kind: EffectKind::NoOp,
effect_detail: TargetEffectDetail::None,
},
],
mode_labels: Vec::new(),
selection: crate::types::game_state::TargetSelectionProgress {
current_slot: 0,
Expand All @@ -6616,14 +6631,16 @@ mod tests {
},
};

crate::game::perf_counters::reset();
let (actions, _spell_costs, _grouped) = legal_actions_full(&state);

assert_eq!(
crate::game::perf_counters::snapshot().state_clone_for_legality,
0
assert!(actions.contains(&GameAction::ChooseTarget { target: None }));
assert!(actions.contains(&GameAction::CancelCast));
assert!(
!actions
.iter()
.any(|action| matches!(action, GameAction::ChooseTarget { target: Some(_) })),
"stale slot targets must not be reissued"
);
assert_eq!(actions, vec![GameAction::ChooseTarget { target: None }]);
}

/// False-positive sweep (CR 103.5 / TL:R 906.6a): the simultaneous
Expand Down
Loading
Loading