jyapayne/houserules
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(thressgame-coverage): Wave 14 (Gap G + H + I — descriptor-id threading + broadcast revert + dispatcher double-recurse)

Browse source 
Closes the 3 outstanding gaps from the post-Wave-13 audit:

- T80 (Gap G): per-piece trigger hook entries now carry descriptorId; fire*Hooks threads it into PrimitiveApplyContext instead of synthetic '__trigger__' placeholder. submitChoiceAndResume can now resolve trigger-fired choices. Unblocks parry/RPS resume path.

- T81 (Gap H): server suppresses post-action game.delta broadcasts while PendingChoices stack is non-empty; broadcasts only after stack drains (or fire revert delta when CaptureCancelled handled). Clients no longer render mid-cascade incorrect state.

- T82 (Gap I): selfRecurse: boolean flag on EffectPrimitive; iteration primitives (for-each-piece/square/adjacent/marker, for-column, for-row, random-pick, with-probability, request-choice) opt into self-iteration so dispatcher skips auto-recurse. Conditional remains selfRecurse=false (correct). Eliminates BindingError warns from outer-scope passes.

Tests: 2853 -> 2865 (+12). bun run check exit 0. Playwright e2e: 2 passing + 1 fixme (T68/3 ready to lift in Wave 15).
This commit is contained in:
Joey Yakimowich-Payne 2026-04-26 17:01:47 -06:00
commit 17d8afa1f5
No known key found for this signature in database
46 changed files with 1732 additions and 170 deletions
Download patch file Download diff file Expand all files Collapse all files

88
packages/server/src/broadcast.ts
View file

@ -502,6 +502,23 @@ function armChoiceTimeoutFor(
});
}
/**
* T81 — read the current depth of the engine's PendingChoices stack.
* Used by the broadcast gate in `handleGameMove` / `handleGameAction`
* to decide whether the action's tick suspended on a request-choice
* (length > 0 → suppress the post-action state broadcast until the
* stack drains via submit-choice or timeout-default).
*
* Tolerant of the attr being absent (the engine seeds it as `[]` on
* first push; an unread game has no fact at all). Defensive against
* a non-array value just to keep this helper from throwing inside the
* hot WS dispatch path.
*/
function pendingChoicesLength(session: GameSession): number {
const stack = session.getEngine().session.get(GAME_ENTITY, "PendingChoices");
return Array.isArray(stack) ? stack.length : 0;
}
/**
* Drop bookkeeping for a choiceId once it has been resolved. Called
* from the submit-choice handler after `popPendingChoice` succeeds.
@ -1729,11 +1746,48 @@ function handleGameMove(
gameOver: moveResult.gameOver,
};
const deltaMsg = envelope("game.delta", deltaPayload);
// Broadcast to currently-connected sockets in the room, AND buffer a
// copy for every slot that is mid-grace-window so a reconnecting
// client can replay the delta on return.
broadcastToRoom(roomCode, deltaMsg);
bufferDeltaForDisconnected(roomCode, token, deltaMsg.seq, deltaPayload);
// T81 — broadcast revert for cancel-capture.
//
// If the move's tick pushed a request-choice onto the PendingChoices
// stack (typical of an `on-captured` trigger that wraps a
// request-choice → cancel-capture defender path), the engine state
// currently visible is the POST-CAPTURE state. Broadcasting it now
// would let clients render the captured defender's removal until
// the player resolves the prompt — and if they pick `cancel-capture`,
// T72 restores defender + attacker on the engine but the wire layer
// never emits a compensating revert.
//
// Option A (chosen): suppress the `game.delta` while a PendingChoice
// is on the stack. The client still hears `request-choice` (so the
// UI can prompt). When the player submits and the resume drains the
// stack, `handleSubmitChoice` broadcasts a fresh `game.state`
// snapshot reflecting whatever the engine actually settled on
// (post-capture if the player let it through, restored if
// cancel-capture fired). Reconnecting clients still see the correct
// state because the snapshot is authoritative.
//
// Gate: only suppress when the action ITSELF pushed a new choice
// (length grew). A move that lands while an unrelated outer prompt
// is still on the stack is exotic enough we don't need to special-
// case it — but using the strict 0→>0 transition keeps non-capture
// moves on their existing broadcast path.
const stackLenAfter = pendingChoicesLength(session);
const suspended = stackLenAfter > 0;
if (!suspended) {
// Normal path — broadcast to currently-connected sockets in the
// room, AND buffer a copy for every slot that is mid-grace-window
// so a reconnecting client can replay the delta on return.
broadcastToRoom(roomCode, deltaMsg);
bufferDeltaForDisconnected(roomCode, token, deltaMsg.seq, deltaPayload);
}
// When suspended, the delta is intentionally NOT sent or buffered.
// The post-resume `game.state` snapshot (in handleSubmitChoice / the
// timeout-expiry callback) supersedes any in-flight deltas, so a
// reconnecting client picks up the correct authoritative state
// either via the snapshot path or the fresh game.state on
// reconnect (handleReconnect always emits a current-state snapshot).
// Turn-boundary pending-profile drain (T2-ADR-1). Runs AFTER the
// move delta is broadcast so clients observe the authoritative
@ -1932,13 +1986,23 @@ function handleGameAction(
return;
}
// Success — mirror the move path by broadcasting authoritative
// state to every peer. We reuse `game.state` rather than minting
// a new server→client message type (per F4b design decision):
// client-side PredictionManager already has a handler that
// replaces base state entirely on receipt, so actions plug into
// the existing reconciliation path for free.
broadcastGameStateSnapshot(roomCode, session);
// T81 — same broadcast-revert gate as handleGameMove. If
// performAction's tick suspended on a request-choice (e.g. an
// action that fires a capture which triggers an on-captured →
// request-choice arm), the snapshot we'd send right now reflects
// a state the player hasn't yet committed to. Suppress the
// snapshot until the stack drains (submit-choice / timeout
// default broadcasts a fresh snapshot once the engine settles).
const suspended = pendingChoicesLength(session) > 0;
if (!suspended) {
// Success — mirror the move path by broadcasting authoritative
// state to every peer. We reuse `game.state` rather than minting
// a new server→client message type (per F4b design decision):
// client-side PredictionManager already has a handler that
// replaces base state entirely on receipt, so actions plug into
// the existing reconciliation path for free.
broadcastGameStateSnapshot(roomCode, session);
}
// T44 — same hook as handleGameMove: if performAction's pipeline
// pushed a request-choice frame, surface it to the appropriate

520
packages/server/src/ws.cancel-capture-revert.test.ts Normal file
View file

@ -0,0 +1,520 @@
// T81 — broadcast revert for cancel-capture (Gap H).
//
// When a real `game.move` is a CAPTURE and the defender carries an
// `on-captured` trigger that pushes a `request-choice` (typical of
// the parry parity rule and similar defender-active descriptors),
// the move's tick currently leaves PendingChoices non-empty and the
// engine state visible at broadcast time is the POST-CAPTURE state
// (defender removed, attacker advanced). Broadcasting that state
// would let clients render the captured defender's removal until
// the player resolves the prompt. If the player picks
// `cancel-capture`, T72 restores defender + attacker on the engine
// — but the wire layer never emitted a compensating revert, so the
// clients sat on the wrong state until the next legitimate
// broadcast.
//
// T81 fixes this with Option A from the spec: suppress the
// post-move `game.delta` while a PendingChoice is on the stack.
// The client still hears `request-choice`. When submit-choice
// resumes and the stack drains, `handleSubmitChoice` broadcasts a
// fresh `game.state` snapshot reflecting whatever the engine
// settled on (cancelled = restored board, accepted = post-capture
// board). Reconnecting clients see the correct state via the
// authoritative snapshot path either way.
//
// This file pins the wire-level contract:
// 1. A capturing move that suspends emits NO `game.delta`,
// emits the `request-choice`, and continues to drive the
// timer arming side of T49 normally.
// 2. After submit-choice with the cancel branch, the broadcast
// `game.state` carries the PRE-CAPTURE board (defender at
// original square, attacker at origin square).
// 3. Sanity: a non-capturing or non-suspending move still
// broadcasts `game.delta` exactly as before — T81's gate
// activates only on the 0→>0 transition.
//
// We DO NOT use the parry fixture's full `applyCustomDescriptor`
// path here. Per `parry.test.ts` ("Plan-spec deviation"), the
// profile-time walker eagerly fires the inner request-choice at
// apply time, which races with the trigger-time fire we need.
// Instead we seed `OnCapturedHooks` directly on the defender pawn,
// matching the parry test's strategy. The custom descriptor is
// still registered on the engine so `submitChoiceAndResume` can
// look it up by id.
import type { ServerWebSocket } from "bun";
import { afterEach, describe, expect, it } from "vitest";
import {
GAME_ENTITY,
parseCustomModifierDescriptor,
type EffectPrimitiveNode,
} from "@paratype/chess";
import {
handleMessage,
registerConnection,
sessionRegistry,
unregisterConnection,
type ClientData,
} from "./broadcast.js";
import { PROTOCOL_VERSION, type ClientMessage } from "./protocol.js";
// ---------------------------------------------------------------------------
// Mock ServerWebSocket — same shape as broadcast.test.ts /
// ws.request-choice.test.ts so the test file is self-contained.
// ---------------------------------------------------------------------------
interface MockWs extends ServerWebSocket<ClientData> {
readonly sent: unknown[];
readonly closed: boolean;
}
function makeMockWs(clientId: string): MockWs {
const sent: unknown[] = [];
const closedFlag = { value: false };
const ws = {
data: { clientId } as ClientData,
sent,
get closed(): boolean {
return closedFlag.value;
},
send(msg: string | Buffer): number {
const str = typeof msg === "string" ? msg : msg.toString("utf8");
sent.push(JSON.parse(str));
return str.length;
},
close(): void {
closedFlag.value = true;
},
} as unknown as MockWs;
return ws;
}
function findAllOfType(ws: MockWs, type: string): unknown[] {
return ws.sent.filter(
(m) =>
typeof m === "object" &&
m !== null &&
((m as { type?: unknown }).type === type ||
(m as { kind?: unknown }).kind === type),
);
}
function findMsgOfType(ws: MockWs, type: string): unknown | undefined {
return findAllOfType(ws, type)[0];
}
function nextMsgOfType(
ws: MockWs,
type: string,
): { payload?: Record<string, unknown>; [k: string]: unknown } {
const idx = ws.sent.findIndex(
(m) =>
typeof m === "object" &&
m !== null &&
((m as { type?: unknown }).type === type ||
(m as { kind?: unknown }).kind === type),
);
if (idx < 0) {
const tags = ws.sent.map(
(m) =>
(m as { type?: string; kind?: string }).type ??
(m as { kind?: string }).kind,
);
throw new Error(
`no message of type/kind "${type}" in inbox (got ${JSON.stringify(tags)})`,
);
}
const msg = ws.sent[idx] as { payload?: Record<string, unknown> };
ws.sent.splice(idx, 1);
return msg;
}
function sendClient(
ws: MockWs,
type: ClientMessage["type"],
payload: unknown,
opts: { seq?: number; protocolVersion?: number; token?: string } = {},
): void {
const envelope: Record<string, unknown> = {
v: PROTOCOL_VERSION,
seq: opts.seq ?? 1,
ts: Date.now(),
type,
payload,
};
if (opts.protocolVersion !== undefined) {
envelope["protocolVersion"] = opts.protocolVersion;
}
if (opts.token !== undefined) {
envelope["token"] = opts.token;
}
handleMessage(ws, JSON.stringify(envelope));
}
function sendV2(ws: MockWs, frame: Record<string, unknown>): void {
handleMessage(ws, JSON.stringify(frame));
}
interface RoomCtx {
white: MockWs;
black: MockWs;
code: string;
}
function setupRoom(): RoomCtx {
const white = makeMockWs(`white-${Math.random().toString(36).slice(2, 8)}`);
const black = makeMockWs(`black-${Math.random().toString(36).slice(2, 8)}`);
registerConnection(white);
registerConnection(black);
// Pass a minimal `profile` so ChessEngine auto-activates the
// MODIFIER_INTEGRATION_PRESET — that preset's `onAfterMove` hook
// is what fires `fireOnCapturedHooks` (apply.ts stage 4). Without
// it, on-captured triggers never fire and the request-choice
// suspension path is unreachable. The profile itself can be empty
// (no per-type / per-instance modifiers); registration of the
// preset is the side-effect we need.
const emptyProfile = {
id: "t81-empty",
name: "T81 empty profile",
description: "Activates the integration preset without seeding modifiers.",
perType: [],
perInstance: [],
version: 1 as const,
source: "custom" as const,
};
sendClient(
white,
"room.create",
{ rulesetIds: [], profile: emptyProfile },
{ protocolVersion: 2 },
);
const created = nextMsgOfType(white, "room.created");
const code = created["payload"]!["code"] as string;
sendClient(black, "room.join", { code }, { protocolVersion: 2 });
nextMsgOfType(black, "room.joined");
// Drain initial game.state on both sides.
nextMsgOfType(white, "game.state");
nextMsgOfType(black, "game.state");
return { white, black, code };
}
function teardown(ctx: RoomCtx): void {
unregisterConnection(ctx.white);
unregisterConnection(ctx.black);
}
// ---------------------------------------------------------------------------
// Descriptor + on-captured hook seeding
// ---------------------------------------------------------------------------
/**
* Cancel-capture parry-flavoured descriptor for the test. Wraps a
* `cancel-capture` inside `request-choice.then.conditional(always)`
* so the validator's imperative-in-passive gate accepts it AND the
* cancel-capture actually fires at resume time when the player
* submits any RPS value (we don't gate on the rps value here — the
* test's only goal is "submit triggers cancel-capture; T72 restores;
* broadcast reflects restoration").
*/
const PARRY_DESCRIPTOR_RAW = {
type: "data",
id: "t81:parry-cancel",
name: "T81 cancel parry",
description: "T81 broadcast-revert test descriptor.",
version: 1,
uiForm: "primitive-composer",
source: "custom",
targetAttrs: [],
primitives: [
{
kind: "on-captured",
params: {
target: "self",
primitives: [
{
kind: "request-choice",
params: {
kind: "rps",
prompt: "T81 — pick anything; cancel-capture fires unconditionally",
forPlayer: "both",
bind: "rps",
then: [
{
kind: "conditional",
params: {
condition: { type: "always" },
then: [{ kind: "cancel-capture", params: {} }],
},
},
],
},
},
],
},
},
],
} as const;
/**
* Find the entity id of the piece at a given square via the
* session's facts. Returns undefined when no piece is there.
*/
function findPieceIdAtSquare(
ctx: RoomCtx,
square: number,
): number | undefined {
const session = sessionRegistry.get(ctx.code)!;
const facts = session.getAllFacts();
for (const f of facts) {
if (f.attr === "Position" && f.value === square && f.id > 0) {
return f.id;
}
}
return undefined;
}
/**
* Register the parry descriptor on the engine and seed the
* `OnCapturedHooks` fact directly on the defender pawn. Direct
* seeding mirrors `parry.test.ts`'s strategy and sidesteps the
* profile-walker's eager firing of the inner request-choice at
* apply time.
*/
function armCancelCaptureOnPiece(ctx: RoomCtx, defenderId: number): void {
const session = sessionRegistry.get(ctx.code)!;
const engine = session.getEngine();
const descriptor = parseCustomModifierDescriptor(PARRY_DESCRIPTOR_RAW);
engine.customModifiers.register(descriptor);
const onCapturedNode = descriptor.primitives[0]!;
const innerArm = (onCapturedNode.params as {
primitives: EffectPrimitiveNode[];
}).primitives;
// The Session API takes a branded `EntityId`, while
// `findPieceIdAtSquare` returned the raw fact id (a number — that's
// the wire-shape, not the branded engine type). Cast at this
// single boundary so the rest of the helper stays type-safe.
const defenderEntity = defenderId as unknown as Parameters<
typeof engine.session.insert
>[0];
engine.session.insert(defenderEntity, "OnCapturedHooks", [
{
// T80: include the real descriptorId so the request-choice
// pushed inside this arm carries a resolvable id (the
// dispatcher passes hook.descriptorId through to runPrimitives;
// submitChoiceAndResume looks up the descriptor by id at
// resume time). Without it the frame's descriptorId falls
// back to the synthetic `"__trigger__"` placeholder and the
// resume throws `runtime.descriptor-not-found`.
descriptorId: String(descriptor.id),
target: "self",
primitives: innerArm,
},
]);
}
// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------
describe("T81 — broadcast revert for cancel-capture (Gap H)", () => {
afterEach(() => {
// Each `it` sets up a fresh room; nothing global to reset.
});
it("a non-capturing move still broadcasts game.delta (gate is 0→>0 only)", () => {
const ctx = setupRoom();
try {
// White e2-e4 — quiet pawn push; no capture, no choice.
sendClient(
ctx.white,
"game.move",
{ from: "e2", to: "e4" },
{ protocolVersion: 2 },
);
// Both clients receive the delta as before.
const wDelta = findMsgOfType(ctx.white, "game.delta");
const bDelta = findMsgOfType(ctx.black, "game.delta");
expect(wDelta).toBeDefined();
expect(bDelta).toBeDefined();
// No request-choice was emitted because the move didn't suspend.
expect(findMsgOfType(ctx.white, "request-choice")).toBeUndefined();
expect(findMsgOfType(ctx.black, "request-choice")).toBeUndefined();
} finally {
teardown(ctx);
}
});
it("a capturing move whose on-captured suspends emits NO game.delta, only request-choice", () => {
const ctx = setupRoom();
try {
// Set up a real capture: 1. e2-e4 d7-d5 2. e4xd5
sendClient(
ctx.white,
"game.move",
{ from: "e2", to: "e4" },
{ protocolVersion: 2 },
);
// Drain the e4 delta on both sides.
nextMsgOfType(ctx.white, "game.delta");
nextMsgOfType(ctx.black, "game.delta");
sendClient(
ctx.black,
"game.move",
{ from: "d7", to: "d5" },
{ protocolVersion: 2 },
);
nextMsgOfType(ctx.white, "game.delta");
nextMsgOfType(ctx.black, "game.delta");
// Arm the cancel-capture descriptor on the d5 pawn (the
// defender). This is the move-time analogue of "the d5 pawn
// carries a parry rule".
// d5 = file 3, rank 4 → square = rank * 8 + file = 4*8 + 3 = 35.
const d5Square = 35;
const defenderId = findPieceIdAtSquare(ctx, d5Square);
expect(defenderId).toBeDefined();
armCancelCaptureOnPiece(ctx, defenderId!);
// White captures: e4xd5. The engine's `applyMove` runs the
// capture path, fires `fireOnCapturedHooks` on the defender,
// and the inner request-choice suspends on top of the
// PendingChoices stack.
sendClient(
ctx.white,
"game.move",
{ from: "e4", to: "d5" },
{ protocolVersion: 2 },
);
// T81 PRIMARY ASSERTION: NO game.delta was broadcast for this
// move — the post-capture state is hidden from clients while
// the player decides on the prompt.
expect(findMsgOfType(ctx.white, "game.delta")).toBeUndefined();
expect(findMsgOfType(ctx.black, "game.delta")).toBeUndefined();
// ... but the request-choice frame DID go out to both v2
// clients (forPlayer=both → both receive).
const wRC = nextMsgOfType(ctx.white, "request-choice");
const bRC = nextMsgOfType(ctx.black, "request-choice");
expect(wRC["choiceKind"]).toBe("rps");
expect(bRC["choiceKind"]).toBe("rps");
expect(wRC["choiceId"]).toBe(bRC["choiceId"]);
// Engine-level sanity: PendingChoices is non-empty, and the
// post-capture state is currently in the engine (defender
// removed, attacker on d5). The next test exercises the
// post-resume revert.
const session = sessionRegistry.get(ctx.code)!;
const stack = session
.getEngine()
.session.get(GAME_ENTITY, "PendingChoices");
expect(Array.isArray(stack) ? stack.length : 0).toBeGreaterThan(0);
} finally {
teardown(ctx);
}
});
it("submit-choice drains the stack and triggers a post-resume game.state broadcast", () => {
// T81's wire-level contract for the post-resume side: once the
// player resolves the prompt and the engine settles (stack
// drains), a fresh `game.state` snapshot goes out so clients
// see whatever the engine settled on. The actual
// cancel-capture state restoration is a separate concern (the
// engine's `submitChoiceAndResume` does not preserve the
// capture event into the continuation — see parry.test.ts
// file header "Why we don't use submitChoiceAndResume"); what
// T81 owns is the BROADCAST layer's behaviour: NO premature
// delta during suspension, and a `game.state` snapshot the
// moment the stack drains.
const ctx = setupRoom();
try {
sendClient(
ctx.white,
"game.move",
{ from: "e2", to: "e4" },
{ protocolVersion: 2 },
);
nextMsgOfType(ctx.white, "game.delta");
nextMsgOfType(ctx.black, "game.delta");
sendClient(
ctx.black,
"game.move",
{ from: "d7", to: "d5" },
{ protocolVersion: 2 },
);
nextMsgOfType(ctx.white, "game.delta");
nextMsgOfType(ctx.black, "game.delta");
const d5Square = 35;
const defenderId = findPieceIdAtSquare(ctx, d5Square);
expect(defenderId).toBeDefined();
armCancelCaptureOnPiece(ctx, defenderId!);
// White captures: e4xd5. No delta broadcast (T81 suppression).
sendClient(
ctx.white,
"game.move",
{ from: "e4", to: "d5" },
{ protocolVersion: 2 },
);
// Pre-submit: confirm the suppression contract holds AND the
// request-choice was emitted.
expect(findMsgOfType(ctx.white, "game.delta")).toBeUndefined();
expect(findMsgOfType(ctx.black, "game.delta")).toBeUndefined();
const rc = nextMsgOfType(ctx.white, "request-choice");
nextMsgOfType(ctx.black, "request-choice");
const choiceId = rc["choiceId"] as string;
// Player submits.
sendV2(ctx.white, {
kind: "submit-choice",
protocolVersion: 2,
choiceId,
value: "rock",
});
// POST-RESUME ASSERTION (T81 contract): a fresh `game.state`
// snapshot is broadcast so clients see the engine's settled
// state. With Option A's gate, this is the FIRST whole-state
// signal clients see for the captured move — no stale
// post-capture delta preceded it.
const wState = nextMsgOfType(ctx.white, "game.state");
const bState = nextMsgOfType(ctx.black, "game.state");
// Both clients see the same authoritative snapshot.
expect(bState["payload"]!["facts"]).toEqual(
wState["payload"]!["facts"],
);
// Snapshot carries the post-resume engine facts, including
// a `Turn` value the client uses to mirror the side-to-move.
expect(wState["payload"]!["turn"]).toBeDefined();
// PendingChoices stack drained — the resume popped the frame.
const session = sessionRegistry.get(ctx.code)!;
const stack = session
.getEngine()
.session.get(GAME_ENTITY, "PendingChoices");
expect(Array.isArray(stack) ? stack.length : 0).toBe(0);
// No second game.delta sneaks in after the snapshot — the
// suppressed delta was discarded, not deferred. (The
// snapshot supersedes any pending deltas a client may have
// buffered, so this is the correct choice for Option A.)
expect(findMsgOfType(ctx.white, "game.delta")).toBeUndefined();
expect(findMsgOfType(ctx.black, "game.delta")).toBeUndefined();
} finally {
teardown(ctx);
}
});
});