diff --git a/.sisyphus/boulder.json b/.sisyphus/boulder.json index 62bce29..a55deaf 100644 --- a/.sisyphus/boulder.json +++ b/.sisyphus/boulder.json @@ -88,7 +88,12 @@ "ses_23470aeb5ffen8G71F1kKLXWE4", "ses_234704ca2ffexGa7YYzkVJ7XQy", "ses_234714154ffePk1XQUSvX1tcat", - "ses_2346243beffeatri9EFKougtx4" + "ses_2346243beffeatri9EFKougtx4", + "ses_2342ca7c3ffe40BnC0Thwejgjg", + "ses_2341a04eeffeiOv4T2Q3rqknl7", + "ses_23413031dffesUm5HBc24bf63F", + "ses_2341377a8ffejodIDjnwXpyIHp", + "ses_23413e9bdffemN8WkabmXJVK5t" ], "plan_name": "thressgame-coverage", "agent": "atlas" diff --git a/packages/chess/src/__fixtures__/parity/kamikaze-real.test.ts b/packages/chess/src/__fixtures__/parity/kamikaze-real.test.ts index 8f7188f..5c2c940 100644 --- a/packages/chess/src/__fixtures__/parity/kamikaze-real.test.ts +++ b/packages/chess/src/__fixtures__/parity/kamikaze-real.test.ts @@ -131,14 +131,15 @@ describe("T78 — kamikaze REAL-pipeline", () => { placePiece(engine, "pawn", "black", 18); // adjacent to d3 - AOE target // Seed OnCaptureHooks directly (mirrors what a fixed - // applyCustomDescriptor would produce). OnCaptureHooks shape: - // Array. + // applyCustomDescriptor would produce). T80: OnCaptureHooks + // entries carry `descriptorId` so the trigger dispatcher can + // thread the real id into runPrimitives (Gap G fix). const onCaptureNode = descriptor.primitives[0]!; const innerArm = (onCaptureNode.params as { primitives: EffectPrimitiveNode[]; }).primitives; engine.session.insert(attacker, "OnCaptureHooks", [ - innerArm, + { descriptorId: descriptor.id, primitives: innerArm }, ] as ChessAttrMap["OnCaptureHooks"]); const cap = engine diff --git a/packages/chess/src/__fixtures__/parity/mind_control-real.test.ts b/packages/chess/src/__fixtures__/parity/mind_control-real.test.ts index bc20742..422e9f5 100644 --- a/packages/chess/src/__fixtures__/parity/mind_control-real.test.ts +++ b/packages/chess/src/__fixtures__/parity/mind_control-real.test.ts @@ -128,8 +128,12 @@ describe("T78 — mind_control REAL-pipeline", () => { expect(top).toBeDefined(); expect(top!.kind).toBe("piece"); expect(top!.forPlayer).toBe("both"); - // V1 sharp edge: trigger-fired choice tagged "__trigger__". - expect(top!.descriptorId).toBe("__trigger__"); + // T80 (Wave 14, Gap G fix) — trigger-fired choices now carry + // the REAL descriptor id threaded from the hook entry through + // `runPrimitives` into the synthesised PrimitiveApplyContext. + // Pre-T80 the dispatcher emitted "__trigger__" as a placeholder + // which broke `submitChoiceAndResume` (descriptor-not-found). + expect(top!.descriptorId).toBe("parity:mind_control"); }); it("AutoChoiceResolver picks target ids deterministically via byId table", () => { @@ -217,9 +221,14 @@ describe("T78 — mind_control REAL-pipeline", () => { popPendingChoice(engine); }); - it("submitChoiceAndResume on the trigger-fired frame hits descriptor-not-found (V1: '__trigger__')", () => { - // Same V1 sharp edge as mr_freeze. The sibling test works - // around it by re-pushing the frame with the registered id. + it("trigger-fired frame carries the REAL descriptor id (T80 Gap G fix)", () => { + // T80 (Wave 14, Gap G fix): the dispatcher now threads the + // owning descriptor's id through trigger dispatch so + // PendingChoice frames pushed inside a trigger arm carry the + // REAL id. `submitChoiceAndResume` resolves the descriptor by + // id (no longer throws `runtime.descriptor-not-found`). + // Pre-T80 this test asserted the throw; post-T80 we pin the + // real-id contract on the suspended frame. const descriptor = parseCustomModifierDescriptor(RAW_FIXTURE); const engine = new ChessEngine({ profile: emptyProfile() }); engine.setRngSeed(66); @@ -240,13 +249,7 @@ describe("T78 — mind_control REAL-pipeline", () => { fireOnRuleActivatedHooks(engine, "parity:mind_control"); const top = peekPendingChoice(engine); expect(top).toBeDefined(); - - // Use a placeholder integer id as the answer — the test - // exercises the early `descriptor-not-found` path which fires - // BEFORE the answer is consumed. - expect(() => - submitChoiceAndResume(engine, top!.choiceId, 1), - ).toThrow(/runtime\.descriptor-not-found/); + expect(top!.descriptorId).toBe("parity:mind_control"); }); it("kings remain unaffected — structural pin (no resolver answer ever points at a king id)", () => { diff --git a/packages/chess/src/__fixtures__/parity/mind_control.test.ts b/packages/chess/src/__fixtures__/parity/mind_control.test.ts index 66b2ab6..f977755 100644 --- a/packages/chess/src/__fixtures__/parity/mind_control.test.ts +++ b/packages/chess/src/__fixtures__/parity/mind_control.test.ts @@ -279,15 +279,14 @@ describe("T66 — mind_control ThressGame parity rule", () => { const rawBlackFrame = stackAfterFire![0]!; expect(rawBlackFrame.kind).toBe("piece"); expect(rawBlackFrame.forPlayer).toBe("both"); - // Trigger-fired choices carry descriptorId = "__trigger__" — - // the dispatcher synthesises a placeholder descriptor ref when - // entering a hook arm (see triggers.ts § "Trigger evaluation - // has no parent descriptor — synthesise a minimal ref"). - // submitChoiceAndResume needs the LIFTED id to look up the - // descriptor tree, so we pop + re-push with the corrected id - // before the LIFO injection step. Mr_freeze pins this same - // dispatcher-id behaviour at line 336 of mr_freeze.test.ts. - expect(rawBlackFrame.descriptorId).toBe("__trigger__"); + // T80 (Wave 14 Gap G fix): trigger-fired choices now carry the + // REAL descriptor id (the lifted descriptor's id), threaded + // from the hook entry through `runPrimitives` into the + // synthesised `PrimitiveApplyContext.descriptor`. Pre-T80 the + // dispatcher emitted "__trigger__" as a placeholder which broke + // `submitChoiceAndResume` (descriptor-not-found). With the fix + // landed, no pop+repush dance is required for the resume path. + expect(rawBlackFrame.descriptorId).toBe(liftedId); expect(rawBlackFrame.triggerPath).toEqual([]); expect(rawBlackFrame.primitiveIndex).toBe(0); diff --git a/packages/chess/src/__fixtures__/parity/mr_freeze-real.test.ts b/packages/chess/src/__fixtures__/parity/mr_freeze-real.test.ts index b9252e3..d24b232 100644 --- a/packages/chess/src/__fixtures__/parity/mr_freeze-real.test.ts +++ b/packages/chess/src/__fixtures__/parity/mr_freeze-real.test.ts @@ -44,10 +44,7 @@ import { } from "../../schema.js"; import { parseCustomModifierDescriptor } from "../../modifiers/custom/schema.js"; import { fireOnRuleActivatedHooks } from "../../modifiers/triggers.js"; -import { - peekPendingChoice, - submitChoiceAndResume, -} from "../../util/pending-choices.js"; +import { peekPendingChoice } from "../../util/pending-choices.js"; import { AutoChoiceResolver } from "../choice-transport/auto-resolver.js"; import type { ModifierProfile } from "../../modifiers/types.js"; import type { EffectPrimitiveNode } from "../../modifiers/primitives/types.js"; @@ -122,26 +119,26 @@ describe("T78 — mr_freeze REAL-pipeline", () => { expect(resolver.resolve(top!)).toBe(4); }); - it("submitChoiceAndResume on the trigger-fired frame hits descriptor-not-found (V1: '__trigger__' placeholder)", () => { - // Documented V1 sharp edge: the dispatcher tags trigger-fired - // choice frames with descriptorId="__trigger__". - // submitChoiceAndResume looks that up in customModifiers and - // throws `runtime.descriptor-not-found`. The sibling test - // works around this by popping + re-pushing with the - // registered descriptor id. + it("trigger-fired frame carries the REAL descriptor id (T80 Gap G fix)", () => { + // T80 (Wave 14, Gap G fix): the dispatcher now threads the + // owning descriptor's id through trigger dispatch (every + // `fire*Hooks` reads `hook.descriptorId` and passes it into + // `runPrimitives`). PendingChoice frames pushed inside a + // trigger arm carry the REAL id, so `submitChoiceAndResume` + // resolves the descriptor by id and walks back into its + // primitive tree to continue execution — no longer throws + // `runtime.descriptor-not-found`. // - // V2 work: thread the owning descriptor's id through trigger - // dispatch so the frame carries the real id. After the fix, - // this test should be tightened to assert the resume runs - // the spawn-marker cascade. + // Pre-T80 this test asserted the throw; post-T80 we pin the + // real-id contract. The follow-up resume cascade (for-row + + // spawn-marker) is exercised by mr_freeze.test.ts via the + // direct `FOR_ROW_PRIMITIVE.apply()` path because Gap I + // (dispatcher double-walk on for-each-adjacent / for-row + // children) is still open and tracked separately (T82). const { engine } = buildAndFire(); const top = peekPendingChoice(engine); expect(top).toBeDefined(); - expect(top!.descriptorId).toBe("__trigger__"); - - expect(() => - submitChoiceAndResume(engine, top!.choiceId, 4), - ).toThrow(/runtime\.descriptor-not-found/); + expect(top!.descriptorId).toBe("parity:mr_freeze"); }); it("expected layout pin: 8 frozen-square markers at column 4 (e1..e8 = squares 4,12,20,28,36,44,52,60)", () => { diff --git a/packages/chess/src/__fixtures__/parity/mr_freeze.test.ts b/packages/chess/src/__fixtures__/parity/mr_freeze.test.ts index 1f04d8b..2b7a2b7 100644 --- a/packages/chess/src/__fixtures__/parity/mr_freeze.test.ts +++ b/packages/chess/src/__fixtures__/parity/mr_freeze.test.ts @@ -330,10 +330,14 @@ describe("T60 — mr_freeze ThressGame parity rule", () => { expect(top).toBeDefined(); expect(top!.kind).toBe("column"); expect(top!.forPlayer).toBe("both"); - // Trigger-fired choices carry descriptorId = "__trigger__" — - // see file docstring § "Resume mechanism" for why this is the - // dispatcher's synthetic placeholder rather than the lifted id. - expect(top!.descriptorId).toBe("__trigger__"); + // T80 (Wave 14 Gap G fix): trigger-fired choices now carry the + // REAL descriptor id (the descriptor that owns the trigger + // hook). Pre-T80 the dispatcher synthesised "__trigger__" as a + // placeholder, which broke `submitChoiceAndResume` because the + // descriptor lookup at resume time failed. The lifted descriptor + // ID flows through `fireOnRuleActivatedHooks` → `runPrimitives` + // → request-choice's PendingChoice.descriptorId. + expect(top!.descriptorId).toBe(liftedId); // The dispatcher entered with triggerPath=[]; the request-choice // is the first primitive in the lifted descriptor's top arm. expect(top!.triggerPath).toEqual([]); diff --git a/packages/chess/src/__fixtures__/parity/parry-real.test.ts b/packages/chess/src/__fixtures__/parity/parry-real.test.ts index 489652f..31c3005 100644 --- a/packages/chess/src/__fixtures__/parity/parry-real.test.ts +++ b/packages/chess/src/__fixtures__/parity/parry-real.test.ts @@ -227,6 +227,7 @@ describe("T72 — parry, REAL-pipeline capture undo (snapshot+restore)", () => { }).primitives; engine.session.insert(blackPawnId!, "OnCapturedHooks", [ { + descriptorId: descriptor.id, target: "self", primitives: innerArm, }, @@ -397,7 +398,7 @@ describe("T72 — parry, REAL-pipeline capture undo (snapshot+restore)", () => { primitives: EffectPrimitiveNode[]; }).primitives; engine.session.insert(blackPawnId!, "OnCapturedHooks", [ - { target: "self", primitives: innerArm }, + { descriptorId: descriptor.id, target: "self", primitives: innerArm }, ] as ChessAttrMap["OnCapturedHooks"]); m = engine diff --git a/packages/chess/src/__fixtures__/parity/parry.test.ts b/packages/chess/src/__fixtures__/parity/parry.test.ts index d4a83b2..74d4b97 100644 --- a/packages/chess/src/__fixtures__/parity/parry.test.ts +++ b/packages/chess/src/__fixtures__/parity/parry.test.ts @@ -233,6 +233,7 @@ function runParryScenario(input: { }).primitives; engine.session.insert(defenderId, "OnCapturedHooks", [ { + descriptorId: descriptor.id, target: "self", primitives: innerArm, }, diff --git a/packages/chess/src/__fixtures__/parity/religious_conversion-real.test.ts b/packages/chess/src/__fixtures__/parity/religious_conversion-real.test.ts index cd8f59f..5101fc9 100644 --- a/packages/chess/src/__fixtures__/parity/religious_conversion-real.test.ts +++ b/packages/chess/src/__fixtures__/parity/religious_conversion-real.test.ts @@ -130,7 +130,7 @@ describe("T78 — religious_conversion REAL-pipeline", () => { ).toThrow(/Binding '\$adj' is not in scope/); }); - it("seeding OnMoveHooks + engine.applyMove enters the real fireOnMoveHooks dispatcher (V1 sharp edge: dispatcher double-walks for-each-adjacent's children)", () => { + it("seeding OnMoveHooks + engine.applyMove enters the real fireOnMoveHooks dispatcher (T80 + selfRecurse=true: hook fires cleanly)", () => { // This test pins the integration-preset wiring: when the // descriptor's hook is seeded directly (bypassing the broken // applyCustomDescriptor walker) and `engine.applyMove` is @@ -158,14 +158,15 @@ describe("T78 — religious_conversion REAL-pipeline", () => { placePiece(engine, "pawn", "black", 29); // f4 - adj to e5 placePiece(engine, "pawn", "black", 44); // e6 - adj to e5 - // Seed the OnMoveHooks fact directly with the inner arm. - // OnMoveHooks shape: Array. + // Seed the OnMoveHooks fact directly with the inner arm. T80: + // OnMoveHooks entries now carry `descriptorId` so the trigger + // dispatcher can thread the real id into runPrimitives (Gap G). const onMoveNode = descriptor.primitives[0]!; const innerArm = (onMoveNode.params as { primitives: EffectPrimitiveNode[]; }).primitives; engine.session.insert(bishopId, "OnMoveHooks", [ - innerArm, + { descriptorId: descriptor.id, primitives: innerArm }, ] as ChessAttrMap["OnMoveHooks"]); // Find a non-capture move for the bishop to e5 (square 36). @@ -173,13 +174,14 @@ describe("T78 — religious_conversion REAL-pipeline", () => { const moveToE5 = moves.find((m) => m.to === 36 && !m.isCapture); expect(moveToE5).toBeDefined(); - // applyMove triggers the V1 sharp edge: for-each-adjacent's - // post-apply child-walk crashes on $adj. The throw escapes - // applyMove (the integration preset's onAfterMove doesn't - // suppress trigger errors). - expect(() => engine.applyMove(moveToE5!)).toThrow( - /Binding '\$adj' is not in scope/, - ); + // applyMove now succeeds — for-each-adjacent's `selfRecurse: + // true` flag prevents the dispatcher's auto child-walk from + // re-running children with the OUTER bindings, and T80 threads + // the real descriptor id through trigger dispatch so nested + // request-choices resume correctly. + const result = engine.applyMove(moveToE5!); + expect(result).toBe("ongoing"); + expect(engine.session.get(bishopId, "Position")).toBe(36); }); it("workaround: register + applyMove succeeds when the descriptor is NOT seeded onto the moved piece", () => { diff --git a/packages/chess/src/modifiers/primitives/for-column.ts b/packages/chess/src/modifiers/primitives/for-column.ts index 6f64de4..f30e7f9 100644 --- a/packages/chess/src/modifiers/primitives/for-column.ts +++ b/packages/chess/src/modifiers/primitives/for-column.ts @@ -126,6 +126,11 @@ const descriptor: EffectPrimitive = { // Children that DO write are visible to manifest/cleanup walks via // `childPrimitives` below. seedsAttrs: [], + // T82 (Gap I) — apply() walks `then` itself with the column binding + // extended into ctx.bindings; the dispatcher must NOT auto-recurse + // (would re-run children with outer scope and throw BindingError on + // `$var` references). See for-row.ts / triggers.ts. + selfRecurse: true, apply(ctx: PrimitiveApplyContext, params: Params): void { // Dedupe + sort ASC for deterministic iteration. Authoring // [3,1,1,5] and [1,3,5] must produce byte-identical traces. diff --git a/packages/chess/src/modifiers/primitives/for-each-adjacent.ts b/packages/chess/src/modifiers/primitives/for-each-adjacent.ts index 5e02150..dcec529 100644 --- a/packages/chess/src/modifiers/primitives/for-each-adjacent.ts +++ b/packages/chess/src/modifiers/primitives/for-each-adjacent.ts @@ -238,6 +238,11 @@ const descriptor: EffectPrimitive = { // writer. Children that DO write are visible to manifest / // cleanup walks via `childPrimitives` below. seedsAttrs: [], + // T82 (Gap I) — apply() walks `then` itself per neighbour with the + // bound square / piece id extended into ctx.bindings; dispatcher + // auto-recursion would re-execute children with the outer scope + // (BindingError on `$var`, double-fire imperatives). + selfRecurse: true, apply(ctx: PrimitiveApplyContext, params: Params): void { // Phase 1 — resolve the centre square. let centreSquare: number; diff --git a/packages/chess/src/modifiers/primitives/for-each-marker.ts b/packages/chess/src/modifiers/primitives/for-each-marker.ts index 7f6f843..dc43ed3 100644 --- a/packages/chess/src/modifiers/primitives/for-each-marker.ts +++ b/packages/chess/src/modifiers/primitives/for-each-marker.ts @@ -148,6 +148,12 @@ const descriptor: EffectPrimitive = { // Children that DO write are visible to manifest/cleanup walks via // `childPrimitives` below. seedsAttrs: [], + // T82 (Gap I) — apply() iterates markers and walks `then` itself + // with the per-iteration markerId bound; dispatcher auto-recursion + // would re-run children with the outer scope (BindingError on + // `$var`, double-fire imperatives). See triggers.ts § auto-recurse + // gate. + selfRecurse: true, apply(ctx: PrimitiveApplyContext, params: Params): void { // Phase 1 — collect matching marker ids. Snapshot semantics: the // list is materialised here; nested primitives that mutate marker diff --git a/packages/chess/src/modifiers/primitives/for-each-piece.ts b/packages/chess/src/modifiers/primitives/for-each-piece.ts index 6de3e45..fb079fc 100644 --- a/packages/chess/src/modifiers/primitives/for-each-piece.ts +++ b/packages/chess/src/modifiers/primitives/for-each-piece.ts @@ -158,6 +158,11 @@ const descriptor: EffectPrimitive = { // Children that DO write are visible to manifest/cleanup walks via // `childPrimitives` below. seedsAttrs: [], + // T82 (Gap I) — apply() iterates pieces and walks `then` itself with + // the per-iteration pieceId bound; the dispatcher must NOT + // auto-recurse (would re-run children with outer scope, throw + // BindingError on `$var` references, and double-fire imperatives). + selfRecurse: true, apply(ctx: PrimitiveApplyContext, params: Params): void { // Phase 1 — collect matching piece ids. Snapshot semantics: the // list is materialised here; nested primitives that mutate piece diff --git a/packages/chess/src/modifiers/primitives/for-each-square.ts b/packages/chess/src/modifiers/primitives/for-each-square.ts index 1c9fa7f..7facfbd 100644 --- a/packages/chess/src/modifiers/primitives/for-each-square.ts +++ b/packages/chess/src/modifiers/primitives/for-each-square.ts @@ -157,6 +157,11 @@ const descriptor: EffectPrimitive = { // Children that DO write are visible to manifest/cleanup walks via // `childPrimitives` below. seedsAttrs: [], + // T82 (Gap I) — apply() walks `then` itself per-square with the + // square index bound; dispatcher auto-recursion would double-execute + // children with the outer scope (BindingError on `$var`, double-fire + // imperatives). See triggers.ts § auto-recurse gate. + selfRecurse: true, apply(ctx: PrimitiveApplyContext, params: Params): void { // Phase 1 — resolve the iteration list. `"all"` (the explicit // literal) and `undefined` both expand to 0..63. An explicit diff --git a/packages/chess/src/modifiers/primitives/for-row.ts b/packages/chess/src/modifiers/primitives/for-row.ts index 999c88e..449971a 100644 --- a/packages/chess/src/modifiers/primitives/for-row.ts +++ b/packages/chess/src/modifiers/primitives/for-row.ts @@ -126,6 +126,12 @@ const descriptor: EffectPrimitive = { // Children that DO write are visible to manifest/cleanup walks via // `childPrimitives` below. seedsAttrs: [], + // T82 (Gap I) — apply() walks `then` itself with the row binding + // extended into ctx.bindings; the dispatcher must NOT auto-recurse + // into childPrimitives() (which would re-execute children with the + // OUTER, un-extended bindings, throwing BindingError on `$var` + // references and double-firing imperatives). + selfRecurse: true, apply(ctx: PrimitiveApplyContext, params: Params): void { // Dedupe + sort ASC for deterministic iteration. Authoring // [3,1,1,5] and [1,3,5] must produce byte-identical traces. diff --git a/packages/chess/src/modifiers/primitives/on-capture.test.ts b/packages/chess/src/modifiers/primitives/on-capture.test.ts index b31af17..183a5c0 100644 --- a/packages/chess/src/modifiers/primitives/on-capture.test.ts +++ b/packages/chess/src/modifiers/primitives/on-capture.test.ts @@ -45,7 +45,9 @@ describe("on-capture primitive — apply()", () => { ON_CAPTURE_PRIMITIVE.apply(ctx, { primitives }); - expect(session.get(ctx.pieceId, "OnCaptureHooks")).toEqual([primitives]); + expect(session.get(ctx.pieceId, "OnCaptureHooks")).toEqual([ + { descriptorId: ctx.descriptor.id, primitives }, + ]); }); it("appends additional hooks to existing OnCaptureHooks", () => { @@ -60,7 +62,10 @@ describe("on-capture primitive — apply()", () => { ON_CAPTURE_PRIMITIVE.apply(ctx, { primitives: first }); ON_CAPTURE_PRIMITIVE.apply(ctx, { primitives: second }); - expect(session.get(ctx.pieceId, "OnCaptureHooks")).toEqual([first, second]); + expect(session.get(ctx.pieceId, "OnCaptureHooks")).toEqual([ + { descriptorId: ctx.descriptor.id, primitives: first }, + { descriptorId: ctx.descriptor.id, primitives: second }, + ]); }); }); diff --git a/packages/chess/src/modifiers/primitives/on-capture.ts b/packages/chess/src/modifiers/primitives/on-capture.ts index c75d652..52353ec 100644 --- a/packages/chess/src/modifiers/primitives/on-capture.ts +++ b/packages/chess/src/modifiers/primitives/on-capture.ts @@ -50,7 +50,10 @@ const descriptor: EffectPrimitive = { ctx.session.insert(ctx.pieceId, "OnCaptureHooks", [ ...existing, - [...params.primitives], + { + descriptorId: ctx.descriptor.id, + primitives: [...params.primitives], + }, ]); }, childPrimitives(params: Params): EffectPrimitiveNode[] { diff --git a/packages/chess/src/modifiers/primitives/on-captured.test.ts b/packages/chess/src/modifiers/primitives/on-captured.test.ts index 73f2b7e..9017c81 100644 --- a/packages/chess/src/modifiers/primitives/on-captured.test.ts +++ b/packages/chess/src/modifiers/primitives/on-captured.test.ts @@ -53,7 +53,9 @@ describe("on-captured primitive — apply() seeding", () => { const hooks = session.get(ctx.pieceId, "OnCapturedHooks") as | ChessAttrMap["OnCapturedHooks"] | undefined; - expect(hooks).toEqual([{ target: "self", primitives }]); + expect(hooks).toEqual([ + { descriptorId: ctx.descriptor.id, target: "self", primitives }, + ]); }); it("seeds with target='attacker'", () => { @@ -67,7 +69,9 @@ describe("on-captured primitive — apply() seeding", () => { const hooks = session.get(ctx.pieceId, "OnCapturedHooks") as | ChessAttrMap["OnCapturedHooks"] | undefined; - expect(hooks).toEqual([{ target: "attacker", primitives }]); + expect(hooks).toEqual([ + { descriptorId: ctx.descriptor.id, target: "attacker", primitives }, + ]); }); it("seeds with target={relation:'ally', filter:{pieceType:'queen'}}", () => { @@ -85,7 +89,9 @@ describe("on-captured primitive — apply() seeding", () => { const hooks = session.get(ctx.pieceId, "OnCapturedHooks") as | ChessAttrMap["OnCapturedHooks"] | undefined; - expect(hooks).toEqual([{ target, primitives }]); + expect(hooks).toEqual([ + { descriptorId: ctx.descriptor.id, target, primitives }, + ]); }); it("seeds with target={squares:[28,35]}", () => { @@ -100,7 +106,9 @@ describe("on-captured primitive — apply() seeding", () => { const hooks = session.get(ctx.pieceId, "OnCapturedHooks") as | ChessAttrMap["OnCapturedHooks"] | undefined; - expect(hooks).toEqual([{ target, primitives }]); + expect(hooks).toEqual([ + { descriptorId: ctx.descriptor.id, target, primitives }, + ]); }); it("appends additional hooks to existing OnCapturedHooks", () => { @@ -122,8 +130,16 @@ describe("on-captured primitive — apply() seeding", () => { | ChessAttrMap["OnCapturedHooks"] | undefined; expect(hooks).toEqual([ - { target: "self", primitives: first }, - { target: "attacker", primitives: second }, + { + descriptorId: ctx.descriptor.id, + target: "self", + primitives: first, + }, + { + descriptorId: ctx.descriptor.id, + target: "attacker", + primitives: second, + }, ]); }); }); diff --git a/packages/chess/src/modifiers/primitives/on-captured.ts b/packages/chess/src/modifiers/primitives/on-captured.ts index 634154f..dbef0a9 100644 --- a/packages/chess/src/modifiers/primitives/on-captured.ts +++ b/packages/chess/src/modifiers/primitives/on-captured.ts @@ -101,7 +101,11 @@ const descriptor: EffectPrimitive = { ctx.session.insert(ctx.pieceId, "OnCapturedHooks", [ ...existing, - { target, primitives: [...params.primitives] }, + { + descriptorId: ctx.descriptor.id, + target, + primitives: [...params.primitives], + }, ]); }, childPrimitives(params: Params): EffectPrimitiveNode[] { diff --git a/packages/chess/src/modifiers/primitives/on-check-delivered.test.ts b/packages/chess/src/modifiers/primitives/on-check-delivered.test.ts index fa3616f..a48aead 100644 --- a/packages/chess/src/modifiers/primitives/on-check-delivered.test.ts +++ b/packages/chess/src/modifiers/primitives/on-check-delivered.test.ts @@ -54,7 +54,7 @@ describe("on-check-delivered primitive — apply()", () => { ON_CHECK_DELIVERED_PRIMITIVE.apply(ctx, { primitives }); expect(session.get(ctx.pieceId, "OnCheckDeliveredHooks")).toEqual([ - primitives, + { descriptorId: ctx.descriptor.id, primitives }, ]); }); @@ -71,8 +71,8 @@ describe("on-check-delivered primitive — apply()", () => { ON_CHECK_DELIVERED_PRIMITIVE.apply(ctx, { primitives: second }); expect(session.get(ctx.pieceId, "OnCheckDeliveredHooks")).toEqual([ - first, - second, + { descriptorId: ctx.descriptor.id, primitives: first }, + { descriptorId: ctx.descriptor.id, primitives: second }, ]); }); }); diff --git a/packages/chess/src/modifiers/primitives/on-check-delivered.ts b/packages/chess/src/modifiers/primitives/on-check-delivered.ts index e6efb6b..57282e0 100644 --- a/packages/chess/src/modifiers/primitives/on-check-delivered.ts +++ b/packages/chess/src/modifiers/primitives/on-check-delivered.ts @@ -64,7 +64,10 @@ const descriptor: EffectPrimitive = { ctx.session.insert(ctx.pieceId, "OnCheckDeliveredHooks", [ ...existing, - [...params.primitives], + { + descriptorId: ctx.descriptor.id, + primitives: [...params.primitives], + }, ]); }, childPrimitives(params: Params): EffectPrimitiveNode[] { diff --git a/packages/chess/src/modifiers/primitives/on-check-received.test.ts b/packages/chess/src/modifiers/primitives/on-check-received.test.ts index 8cb66e7..332dde8 100644 --- a/packages/chess/src/modifiers/primitives/on-check-received.test.ts +++ b/packages/chess/src/modifiers/primitives/on-check-received.test.ts @@ -48,7 +48,7 @@ describe("on-check-received primitive — apply()", () => { ON_CHECK_RECEIVED_PRIMITIVE.apply(ctx, { primitives }); expect(session.get(ctx.pieceId, "OnCheckReceivedHooks")).toEqual([ - primitives, + { descriptorId: ctx.descriptor.id, primitives }, ]); }); @@ -68,8 +68,8 @@ describe("on-check-received primitive — apply()", () => { ON_CHECK_RECEIVED_PRIMITIVE.apply(ctx, { primitives: second }); expect(session.get(ctx.pieceId, "OnCheckReceivedHooks")).toEqual([ - first, - second, + { descriptorId: ctx.descriptor.id, primitives: first }, + { descriptorId: ctx.descriptor.id, primitives: second }, ]); }); }); diff --git a/packages/chess/src/modifiers/primitives/on-check-received.ts b/packages/chess/src/modifiers/primitives/on-check-received.ts index 5fde832..a7c86a3 100644 --- a/packages/chess/src/modifiers/primitives/on-check-received.ts +++ b/packages/chess/src/modifiers/primitives/on-check-received.ts @@ -61,7 +61,10 @@ const descriptor: EffectPrimitive = { ctx.session.insert(ctx.pieceId, "OnCheckReceivedHooks", [ ...existing, - [...params.primitives], + { + descriptorId: ctx.descriptor.id, + primitives: [...params.primitives], + }, ]); }, childPrimitives(params: Params): EffectPrimitiveNode[] { diff --git a/packages/chess/src/modifiers/primitives/on-damaged.test.ts b/packages/chess/src/modifiers/primitives/on-damaged.test.ts index 4c93484..9836899 100644 --- a/packages/chess/src/modifiers/primitives/on-damaged.test.ts +++ b/packages/chess/src/modifiers/primitives/on-damaged.test.ts @@ -45,7 +45,9 @@ describe("on-damaged primitive — apply()", () => { ON_DAMAGED_PRIMITIVE.apply(ctx, { primitives }); - expect(session.get(ctx.pieceId, "OnDamagedHooks")).toEqual([primitives]); + expect(session.get(ctx.pieceId, "OnDamagedHooks")).toEqual([ + { descriptorId: ctx.descriptor.id, primitives }, + ]); }); it("appends additional hooks to existing OnDamagedHooks", () => { @@ -60,7 +62,10 @@ describe("on-damaged primitive — apply()", () => { ON_DAMAGED_PRIMITIVE.apply(ctx, { primitives: first }); ON_DAMAGED_PRIMITIVE.apply(ctx, { primitives: second }); - expect(session.get(ctx.pieceId, "OnDamagedHooks")).toEqual([first, second]); + expect(session.get(ctx.pieceId, "OnDamagedHooks")).toEqual([ + { descriptorId: ctx.descriptor.id, primitives: first }, + { descriptorId: ctx.descriptor.id, primitives: second }, + ]); }); }); diff --git a/packages/chess/src/modifiers/primitives/on-damaged.ts b/packages/chess/src/modifiers/primitives/on-damaged.ts index 97f5a04..8a083bf 100644 --- a/packages/chess/src/modifiers/primitives/on-damaged.ts +++ b/packages/chess/src/modifiers/primitives/on-damaged.ts @@ -50,7 +50,10 @@ const descriptor: EffectPrimitive = { ctx.session.insert(ctx.pieceId, "OnDamagedHooks", [ ...existing, - [...params.primitives], + { + descriptorId: ctx.descriptor.id, + primitives: [...params.primitives], + }, ]); }, childPrimitives(params: Params): EffectPrimitiveNode[] { diff --git a/packages/chess/src/modifiers/primitives/on-move.test.ts b/packages/chess/src/modifiers/primitives/on-move.test.ts index 0a07c22..487a48c 100644 --- a/packages/chess/src/modifiers/primitives/on-move.test.ts +++ b/packages/chess/src/modifiers/primitives/on-move.test.ts @@ -54,7 +54,9 @@ describe("on-move primitive — apply() seeds hook attr", () => { ON_MOVE_PRIMITIVE.apply(ctx, { primitives }); - expect(session.get(ctx.pieceId, "OnMoveHooks")).toEqual([primitives]); + expect(session.get(ctx.pieceId, "OnMoveHooks")).toEqual([ + { descriptorId: ctx.descriptor.id, primitives }, + ]); }); it("fires on captures too — hook list accumulates across apply calls", () => { @@ -70,8 +72,8 @@ describe("on-move primitive — apply() seeds hook attr", () => { ON_MOVE_PRIMITIVE.apply(ctx, { primitives: captureHook }); expect(session.get(ctx.pieceId, "OnMoveHooks")).toEqual([ - quietMoveHook, - captureHook, + { descriptorId: ctx.descriptor.id, primitives: quietMoveHook }, + { descriptorId: ctx.descriptor.id, primitives: captureHook }, ]); }); @@ -85,7 +87,9 @@ describe("on-move primitive — apply() seeds hook attr", () => { ]; ON_MOVE_PRIMITIVE.apply(rookCtx, { primitives: rookHook }); - expect(session.get(rookId, "OnMoveHooks")).toEqual([rookHook]); + expect(session.get(rookId, "OnMoveHooks")).toEqual([ + { descriptorId: rookCtx.descriptor.id, primitives: rookHook }, + ]); // The king (which did not have the primitive applied) must NOT have a hook. expect(session.get(kingCtx.pieceId, "OnMoveHooks")).toBeUndefined(); }); @@ -102,7 +106,9 @@ describe("on-move primitive — apply() seeds hook attr", () => { // Only the piece the primitive was applied to gets the hook seeded; // an unrelated "opponent" piece gets nothing — mirrors "static piece // doesn't fire" semantics at the seeding layer. - expect(session.get(ctx.pieceId, "OnMoveHooks")).toEqual([primitives]); + expect(session.get(ctx.pieceId, "OnMoveHooks")).toEqual([ + { descriptorId: ctx.descriptor.id, primitives }, + ]); expect(session.get(opponentId, "OnMoveHooks")).toBeUndefined(); }); }); diff --git a/packages/chess/src/modifiers/primitives/on-move.ts b/packages/chess/src/modifiers/primitives/on-move.ts index 64fe4b8..507443e 100644 --- a/packages/chess/src/modifiers/primitives/on-move.ts +++ b/packages/chess/src/modifiers/primitives/on-move.ts @@ -60,7 +60,10 @@ const descriptor: EffectPrimitive = { ctx.session.insert(ctx.pieceId, "OnMoveHooks", [ ...existing, - [...params.primitives], + { + descriptorId: ctx.descriptor.id, + primitives: [...params.primitives], + }, ]); }, childPrimitives(params: Params): EffectPrimitiveNode[] { diff --git a/packages/chess/src/modifiers/primitives/on-moved-onto-square.test.ts b/packages/chess/src/modifiers/primitives/on-moved-onto-square.test.ts index 27baddf..d217d43 100644 --- a/packages/chess/src/modifiers/primitives/on-moved-onto-square.test.ts +++ b/packages/chess/src/modifiers/primitives/on-moved-onto-square.test.ts @@ -58,6 +58,7 @@ describe("on-moved-onto-square primitive — apply()", () => { expect(session.get(ctx.pieceId, "OnMovedOntoSquareHooks")).toEqual([ { + descriptorId: ctx.descriptor.id, filter: { kind: "squares", squares: [27, 28, 35, 36] }, primitives, }, @@ -80,6 +81,7 @@ describe("on-moved-onto-square primitive — apply()", () => { expect(session.get(ctx.pieceId, "OnMovedOntoSquareHooks")).toEqual([ { + descriptorId: ctx.descriptor.id, filter: { kind: "predicate", rank: 7 }, primitives, }, @@ -106,10 +108,12 @@ describe("on-moved-onto-square primitive — apply()", () => { expect(session.get(ctx.pieceId, "OnMovedOntoSquareHooks")).toEqual([ { + descriptorId: ctx.descriptor.id, filter: { kind: "squares", squares: [28] }, primitives: firstPrims, }, { + descriptorId: ctx.descriptor.id, filter: { kind: "predicate", file: 4 }, primitives: secondPrims, }, diff --git a/packages/chess/src/modifiers/primitives/on-moved-onto-square.ts b/packages/chess/src/modifiers/primitives/on-moved-onto-square.ts index 1ac759c..79f8f56 100644 --- a/packages/chess/src/modifiers/primitives/on-moved-onto-square.ts +++ b/packages/chess/src/modifiers/primitives/on-moved-onto-square.ts @@ -114,6 +114,7 @@ const descriptor: EffectPrimitive = { } const next: OnMovedOntoSquareHook = { + descriptorId: ctx.descriptor.id, filter, primitives: [...params.primitives], }; diff --git a/packages/chess/src/modifiers/primitives/on-promotion.test.ts b/packages/chess/src/modifiers/primitives/on-promotion.test.ts index 86245c2..6ec4550 100644 --- a/packages/chess/src/modifiers/primitives/on-promotion.test.ts +++ b/packages/chess/src/modifiers/primitives/on-promotion.test.ts @@ -49,7 +49,9 @@ describe("on-promotion primitive — apply()", () => { ON_PROMOTION_PRIMITIVE.apply(ctx, { primitives }); - expect(session.get(ctx.pieceId, "OnPromotionHooks")).toEqual([primitives]); + expect(session.get(ctx.pieceId, "OnPromotionHooks")).toEqual([ + { descriptorId: ctx.descriptor.id, primitives }, + ]); }); it("stacks across multiple apply calls", () => { @@ -65,8 +67,8 @@ describe("on-promotion primitive — apply()", () => { ON_PROMOTION_PRIMITIVE.apply(ctx, { primitives: second }); expect(session.get(ctx.pieceId, "OnPromotionHooks")).toEqual([ - first, - second, + { descriptorId: ctx.descriptor.id, primitives: first }, + { descriptorId: ctx.descriptor.id, primitives: second }, ]); }); }); diff --git a/packages/chess/src/modifiers/primitives/on-promotion.ts b/packages/chess/src/modifiers/primitives/on-promotion.ts index 79d6f9d..12dcd16 100644 --- a/packages/chess/src/modifiers/primitives/on-promotion.ts +++ b/packages/chess/src/modifiers/primitives/on-promotion.ts @@ -63,7 +63,10 @@ const descriptor: EffectPrimitive = { ctx.session.insert(ctx.pieceId, "OnPromotionHooks", [ ...existing, - [...params.primitives], + { + descriptorId: ctx.descriptor.id, + primitives: [...params.primitives], + }, ]); }, childPrimitives(params: Params): EffectPrimitiveNode[] { diff --git a/packages/chess/src/modifiers/primitives/on-turn-end.test.ts b/packages/chess/src/modifiers/primitives/on-turn-end.test.ts index 7d4a594..0fc321e 100644 --- a/packages/chess/src/modifiers/primitives/on-turn-end.test.ts +++ b/packages/chess/src/modifiers/primitives/on-turn-end.test.ts @@ -46,7 +46,7 @@ describe("on-turn-end primitive — apply()", () => { ON_TURN_END_PRIMITIVE.apply(ctx, { color: "both", primitives }); expect(session.get(ctx.pieceId, "OnTurnEndHooks")).toEqual([ - { color: "both", primitives }, + { descriptorId: ctx.descriptor.id, color: "both", primitives }, ]); }); @@ -63,8 +63,8 @@ describe("on-turn-end primitive — apply()", () => { ON_TURN_END_PRIMITIVE.apply(ctx, { color: "white", primitives: second }); expect(session.get(ctx.pieceId, "OnTurnEndHooks")).toEqual([ - { color: "both", primitives: first }, - { color: "white", primitives: second }, + { descriptorId: ctx.descriptor.id, color: "both", primitives: first }, + { descriptorId: ctx.descriptor.id, color: "white", primitives: second }, ]); }); }); diff --git a/packages/chess/src/modifiers/primitives/on-turn-end.ts b/packages/chess/src/modifiers/primitives/on-turn-end.ts index 79d71f8..382d15f 100644 --- a/packages/chess/src/modifiers/primitives/on-turn-end.ts +++ b/packages/chess/src/modifiers/primitives/on-turn-end.ts @@ -67,7 +67,11 @@ const descriptor: EffectPrimitive = { // turn's color at fire-time. Schema attr shape extended in T12. ctx.session.insert(ctx.pieceId, "OnTurnEndHooks", [ ...existing, - { color: params.color, primitives: [...params.primitives] }, + { + descriptorId: ctx.descriptor.id, + color: params.color, + primitives: [...params.primitives], + }, ]); }, childPrimitives(params: Params): EffectPrimitiveNode[] { diff --git a/packages/chess/src/modifiers/primitives/on-turn-start.test.ts b/packages/chess/src/modifiers/primitives/on-turn-start.test.ts index f864ff6..41cb9b8 100644 --- a/packages/chess/src/modifiers/primitives/on-turn-start.test.ts +++ b/packages/chess/src/modifiers/primitives/on-turn-start.test.ts @@ -45,7 +45,9 @@ describe("on-turn-start primitive — apply()", () => { ON_TURN_START_PRIMITIVE.apply(ctx, { primitives }); - expect(session.get(ctx.pieceId, "OnTurnStartHooks")).toEqual([primitives]); + expect(session.get(ctx.pieceId, "OnTurnStartHooks")).toEqual([ + { descriptorId: ctx.descriptor.id, primitives }, + ]); }); it("appends additional hooks to existing OnTurnStartHooks", () => { @@ -60,7 +62,10 @@ describe("on-turn-start primitive — apply()", () => { ON_TURN_START_PRIMITIVE.apply(ctx, { primitives: first }); ON_TURN_START_PRIMITIVE.apply(ctx, { primitives: second }); - expect(session.get(ctx.pieceId, "OnTurnStartHooks")).toEqual([first, second]); + expect(session.get(ctx.pieceId, "OnTurnStartHooks")).toEqual([ + { descriptorId: ctx.descriptor.id, primitives: first }, + { descriptorId: ctx.descriptor.id, primitives: second }, + ]); }); }); diff --git a/packages/chess/src/modifiers/primitives/on-turn-start.ts b/packages/chess/src/modifiers/primitives/on-turn-start.ts index 645fbd3..6a66a2a 100644 --- a/packages/chess/src/modifiers/primitives/on-turn-start.ts +++ b/packages/chess/src/modifiers/primitives/on-turn-start.ts @@ -51,7 +51,10 @@ const descriptor: EffectPrimitive = { ctx.session.insert(ctx.pieceId, "OnTurnStartHooks", [ ...existing, - [...params.primitives], + { + descriptorId: ctx.descriptor.id, + primitives: [...params.primitives], + }, ]); }, childPrimitives(params: Params): EffectPrimitiveNode[] { diff --git a/packages/chess/src/modifiers/primitives/random-pick.ts b/packages/chess/src/modifiers/primitives/random-pick.ts index 2b36408..d98e0af 100644 --- a/packages/chess/src/modifiers/primitives/random-pick.ts +++ b/packages/chess/src/modifiers/primitives/random-pick.ts @@ -137,6 +137,12 @@ const descriptor: EffectPrimitive = { // extension), not a writer. Children that DO write are visible to // manifest/cleanup walks via `childPrimitives` below. seedsAttrs: [], + // T82 (Gap I) — apply() draws a value, binds it, and walks `then` + // itself with the binding extended; dispatcher auto-recursion would + // re-execute `then` with the OUTER scope (BindingError on the + // `$var` referencing the picked value, plus a second imperative + // pass that wasn't intended). + selfRecurse: true, apply(ctx: PrimitiveApplyContext, params: Params): void { // Defensive empty-array guard. The schema's `.min(1)` clamp // rejects authored-empty `from`, but the param resolver (T12) diff --git a/packages/chess/src/modifiers/primitives/request-choice.ts b/packages/chess/src/modifiers/primitives/request-choice.ts index f50c753..bd47a64 100644 --- a/packages/chess/src/modifiers/primitives/request-choice.ts +++ b/packages/chess/src/modifiers/primitives/request-choice.ts @@ -231,6 +231,17 @@ const descriptor: EffectPrimitive = { // declared `seedsAttrs` (the consumer is registered in apply.ts // already — see registerAttrConsumer("PendingChoices")). seedsAttrs: [], + // T82 (Gap I) — apply() SUSPENDS execution (throws + // SuspendedExecution); the continuation `then` is NOT walked at + // apply() time but later by T46's resume mechanism after the + // player answers. The dispatcher catches the throw and `return`s + // before reaching the auto-recurse block, so in practice + // selfRecurse is never consulted on the suspending path. Setting + // it true is defence-in-depth for any future code path that + // bypasses the throw (e.g. a synthetic resume that calls apply() + // without throwing) — the continuation must remain a one-shot + // resume target, not a dispatcher-driven re-execution. + selfRecurse: true, apply(ctx: PrimitiveApplyContext, params: Params): void { // Phase 1 — derive a deterministic choice id. nextInt advances // the persistent RngStream by 1, so the id is reproducible from diff --git a/packages/chess/src/modifiers/primitives/types.ts b/packages/chess/src/modifiers/primitives/types.ts index 40c11ca..43587d1 100644 --- a/packages/chess/src/modifiers/primitives/types.ts +++ b/packages/chess/src/modifiers/primitives/types.ts @@ -286,6 +286,31 @@ export interface EffectPrimitive { readonly longDescription?: string; /** One or more worked examples shown under the explanation. */ readonly examples?: readonly PrimitiveDocExample[]; + /** + * When true, the primitive's apply() handles its own descent into + * nested primitive arrays (e.g. for-each-piece walks `then` itself + * with extended bindings). The dispatcher (`runPrimitives` in + * `triggers.ts`) MUST NOT auto-recurse into the `childPrimitives()` + * result for such primitives; doing so would double-execute the + * children — once with correct (extended) bindings from inside + * apply(), again with the OUTER bindings from the dispatcher's + * standard child-traversal pass. The outer-binding pass throws + * `BindingError` because the iteration's `$var` is unbound at + * outer scope (Gap I). + * + * Default: false. When false, the dispatcher uses + * `childPrimitives()` to discover nested primitive lists and runs + * them automatically with the SAME bindings the parent saw — the + * historical contract that primitives like `conditional` rely on + * (conditional doesn't run its branches at apply() time; the + * dispatcher does it via auto-recursion). + * + * `childPrimitives()` is still consulted regardless of + * `selfRecurse` — manifest / cleanup / validator walkers use it + * to discover what attrs the descendant primitives seed; only + * the dispatcher's RUN-time auto-recursion is gated. + */ + readonly selfRecurse?: boolean; } export type { Session }; diff --git a/packages/chess/src/modifiers/primitives/with-probability.ts b/packages/chess/src/modifiers/primitives/with-probability.ts index 6c005a4..2635da0 100644 --- a/packages/chess/src/modifiers/primitives/with-probability.ts +++ b/packages/chess/src/modifiers/primitives/with-probability.ts @@ -133,6 +133,12 @@ const descriptor: EffectPrimitive = { // not a writer. Nested arms that DO write are visible to manifest / // cleanup walks via `childPrimitives` below. seedsAttrs: [], + // T82 (Gap I) — apply() draws once and runs EITHER `then` OR `else` + // (never both). Dispatcher auto-recursion would walk + // `[...then, ...else]` from childPrimitives() and run BOTH branches + // unconditionally, defeating the probability gate AND double-firing + // the chosen branch. + selfRecurse: true, apply(ctx: PrimitiveApplyContext, params: Params): void { // Phase 1 — draw. ALWAYS call engine.rng().next() so the // persistent stream advances regardless of branch outcome. Two diff --git a/packages/chess/src/modifiers/triggers.gap-i.test.ts b/packages/chess/src/modifiers/triggers.gap-i.test.ts new file mode 100644 index 0000000..ca26ab2 --- /dev/null +++ b/packages/chess/src/modifiers/triggers.gap-i.test.ts @@ -0,0 +1,294 @@ +/** + * T82 (Gap I) — dispatcher double-recurse regression tests. + * + * Before T82, the dispatcher (`runPrimitives` in `triggers.ts`) + * unconditionally walked `primitive.childPrimitives(node.params)` + * after calling `primitive.apply(ctx, params)`. For the iteration + * primitives (for-row / for-each-piece / random-pick / …), + * apply() ALREADY iterates the nested `then` list with the + * per-iteration binding extended into `ctx.bindings`. So the + * dispatcher's auto-recursion was a SECOND pass over the same + * children — but with the OUTER bindings, where the iteration + * variable (e.g. `$r` for for-row) is unbound. That second pass + * either threw `BindingError` (when children referenced `$var`) + * or silently double-fired imperatives (when children were + * binding-free). + * + * The fix: a `selfRecurse: boolean` flag on the EffectPrimitive + * descriptor. Iteration / RNG-binding / branching primitives + * (for-*, random-pick, with-probability, request-choice) declare + * `selfRecurse: true`; the dispatcher SKIPS its auto-recursion + * step for them. `conditional` keeps the historical behaviour + * (selfRecurse undefined ⇒ false ⇒ dispatcher walks both branches + * — but conditional is a HOOK SEEDER, not an evaluator, so that + * walk is just descriptor traversal, not double-execution). + * + * These tests exercise the dispatcher entry point (`runPrimitives`) + * — NOT the primitive's apply() directly — so the auto-recurse + * path is reachable. A direct apply() call would bypass the + * dispatcher and miss the bug entirely. + */ +import { describe, expect, it } from "vitest"; +import { ChessEngine } from "../engine.js"; +import { runPrimitives } from "./triggers.js"; +import type { EffectPrimitiveNode } from "./primitives/types.js"; +import "./primitives/index.js"; + +describe("Gap I — dispatcher double-recurse fix (T82)", () => { + it("for-row binds $var:'r' correctly and does NOT throw BindingError on outer pass", () => { + // The historical bug: apply() runs `then` once with `r` bound + // (good); the dispatcher then walks `childPrimitives()` → + // `[...then]` again with the OUTER bindings (no `r` defined), + // and the inner `add-to-attribute(delta: $var:'r')` resolver + // throws BindingError. Correct behaviour: only ONE pass, with + // bindings. + const engine = new ChessEngine(); + const pieceId = engine.session.nextId(); + const nodes: EffectPrimitiveNode[] = [ + { + kind: "for-row", + params: { + rows: [1, 2, 3], + bind: "r", + then: [ + { + kind: "add-to-attribute", + params: { + attr: "AttackBonus", + delta: { $var: "r" }, + }, + }, + ], + }, + }, + ]; + + // Must NOT throw — historically threw `BindingError: $var 'r' + // is unbound` on the second (outer-bindings) pass. + expect(() => { + runPrimitives(engine, pieceId, nodes, 1); + }).not.toThrow(); + + // Sum must match a SINGLE iteration sweep (1+2+3=6), proving + // the dispatcher did NOT also re-run `then` with outer scope + // (which would either throw OR double the sum if it didn't). + expect(engine.session.get(pieceId, "AttackBonus")).toBe(6); + }); + + it("for-each-piece binds $var:'p' and executes inner primitives EXACTLY ONCE per piece", () => { + // Set up two pieces with PieceType so for-each-piece sees them. + const engine = new ChessEngine(); + const pieceA = engine.session.nextId(); + const pieceB = engine.session.nextId(); + engine.session.insert(pieceA, "PieceType", "rook"); + engine.session.insert(pieceA, "Color", "white"); + engine.session.insert(pieceB, "PieceType", "bishop"); + engine.session.insert(pieceB, "Color", "white"); + + const outerCaller = engine.session.nextId(); + + // Inside the loop, add 1 to ShieldCharges on the ITERATED piece + // (referenced via $var:'p'). After two iterations, both pieces + // should have ShieldCharges=1. If the dispatcher double-recursed + // with the OUTER scope, the second pass would either throw + // (BindingError on `target: { $var: 'p' }`) or — if it somehow + // resolved — write to a different target entirely. Either way + // the count would be wrong. + // set-piece-attr supports `target: { $var: 'p' }` (writes to + // the bound piece). add-to-attribute would NOT work here — it + // hardcodes ctx.pieceId as the write target. Using set-piece-attr + // with target=$var gives us per-piece observability. + const nodes: EffectPrimitiveNode[] = [ + { + kind: "for-each-piece", + params: { + filter: { color: "white" }, + bind: "p", + then: [ + { + kind: "set-piece-attr", + params: { + target: { $var: "p" }, + attr: "ShieldCharges", + value: 1, + }, + }, + ], + }, + }, + ]; + + expect(() => { + runPrimitives(engine, outerCaller, nodes, 1); + }).not.toThrow(); + + // Each piece visited exactly once. If the dispatcher had + // double-recursed with the OUTER scope (no `p` binding), the + // resolver would have thrown BindingError on `target: $var:'p'`, + // failing the not.toThrow assertion. The exact-1 value here + // proves the inner pass DID resolve `$p` (and ran exactly once). + expect(engine.session.get(pieceA, "ShieldCharges")).toBe(1); + expect(engine.session.get(pieceB, "ShieldCharges")).toBe(1); + }); + + it("random-pick binds the picked value and runs `then` exactly once", () => { + // random-pick draws ONE value, binds it, runs `then` once. + // Pre-fix, the dispatcher walked `then` a second time with the + // outer scope; if the inner primitive used `$var:'pick'` as a + // delta value, the second pass threw BindingError. Even if the + // body was binding-free, the imperative would fire twice. + const engine = new ChessEngine(); + const pieceId = engine.session.nextId(); + + const nodes: EffectPrimitiveNode[] = [ + { + kind: "random-pick", + params: { + from: [5], + bind: "pick", + then: [ + { + kind: "add-to-attribute", + params: { + attr: "ShieldCharges", + delta: { $var: "pick" }, + }, + }, + ], + }, + }, + ]; + + expect(() => { + runPrimitives(engine, pieceId, nodes, 1); + }).not.toThrow(); + + // Single pass with `pick` = 5 → final value 5. A double pass + // would either throw (BindingError on the outer scope's missing + // `$pick`) or, with a fallback, the value would be wrong. + expect(engine.session.get(pieceId, "ShieldCharges")).toBe(5); + }); + + it("with-probability runs ONLY the chosen branch (not both via auto-recurse)", () => { + // p=1 always takes `then`. Pre-fix, the dispatcher walked + // childPrimitives() = [...then, ...else] AFTER apply() ran + // the chosen branch — defeating the probability gate AND + // double-firing the chosen branch. + const engine = new ChessEngine(); + const pieceId = engine.session.nextId(); + + const nodes: EffectPrimitiveNode[] = [ + { + kind: "with-probability", + params: { + p: 1, + then: [ + { + kind: "add-to-attribute", + params: { attr: "AttackBonus", delta: 7 }, + }, + ], + else: [ + { + kind: "add-to-attribute", + params: { attr: "AttackBonus", delta: 100 }, + }, + ], + }, + }, + ]; + + runPrimitives(engine, pieceId, nodes, 1); + + // Only `then` ran (delta=7). If `else` had ALSO run via the + // auto-recurse, we'd see 107. + expect(engine.session.get(pieceId, "AttackBonus")).toBe(7); + }); + + it("conditional still recurses correctly (selfRecurse: false)", () => { + // conditional is NOT in the iteration family — its apply() + // SEEDS a hook (ConditionalHooks) and never executes the + // branches at apply() time. The dispatcher's auto-recurse IS + // the right way to walk into nested validators / manifest + // checks. Ensure the dispatcher still descends into + // `then`/`else` for conditional (no regression on the historical + // contract). + const engine = new ChessEngine(); + const pieceId = engine.session.nextId(); + + // The inner `add-to-attribute` only fires via the auto-recurse + // descent — conditional.apply() does NOT execute branches, it + // just seeds the hook. So observing AttackBonus=1 after + // runPrimitives proves the dispatcher walked into `then`. + const nodes: EffectPrimitiveNode[] = [ + { + kind: "conditional", + params: { + condition: { type: "always" }, + then: [ + { + kind: "add-to-attribute", + params: { attr: "AttackBonus", delta: 1 }, + }, + ], + }, + }, + ]; + + runPrimitives(engine, pieceId, nodes, 1); + + // ConditionalHooks seeded by apply() AND the inner primitive + // ran via the dispatcher's auto-recurse pass. AttackBonus=1 + // proves the second. + expect(engine.session.get(pieceId, "AttackBonus")).toBe(1); + expect( + engine.session.get(pieceId, "ConditionalHooks"), + ).toBeDefined(); + }); + + it("nested for-row inside for-column (2D rectangle) binds both vars without doubling", () => { + // Stress test: nested iteration. for-column binds `c`, inner + // for-row binds `r`, leaf `add-to-attribute` uses BOTH via a + // composite. Pre-fix, EACH level's auto-recurse pass would + // throw BindingError on its respective unbound `$var`. Plus + // any successful pass would multiply the iteration count. + const engine = new ChessEngine(); + const pieceId = engine.session.nextId(); + + const nodes: EffectPrimitiveNode[] = [ + { + kind: "for-column", + params: { + columns: [0, 1], + bind: "c", + then: [ + { + kind: "for-row", + params: { + rows: [0, 1, 2], + bind: "r", + then: [ + { + kind: "add-to-attribute", + params: { + attr: "ShieldCharges", + delta: 1, + }, + }, + ], + }, + }, + ], + }, + }, + ]; + + expect(() => { + runPrimitives(engine, pieceId, nodes, 1); + }).not.toThrow(); + + // 2 columns × 3 rows = 6 iterations, each delta=1 ⇒ total 6. + // A double-recurse at either level would yield 12, 18, 24, etc. + expect(engine.session.get(pieceId, "ShieldCharges")).toBe(6); + }); +}); diff --git a/packages/chess/src/modifiers/triggers.test.ts b/packages/chess/src/modifiers/triggers.test.ts index 0ede508..5509b5e 100644 --- a/packages/chess/src/modifiers/triggers.test.ts +++ b/packages/chess/src/modifiers/triggers.test.ts @@ -70,12 +70,15 @@ describe("on-turn-start triggers", () => { // by 1 each turn. const whiteQueen = findPiece(engine, 3); // d1 engine.session.insert(whiteQueen, "OnTurnStartHooks", [ - [ - { - kind: "add-to-attribute", - params: { attr: "RangeBonus", delta: 1 }, - }, - ], + { + descriptorId: "__test_trigger_test__", + primitives: [ + { + kind: "add-to-attribute", + params: { attr: "RangeBonus", delta: 1 }, + }, + ], + }, ]); // Move e2-e4. After this move, the next turn (black) starts. Our @@ -107,12 +110,15 @@ describe("on-turn-start triggers", () => { // Hook on the WHITE queen. const whiteQueen = findPiece(engine, 3); engine.session.insert(whiteQueen, "OnTurnStartHooks", [ - [ - { - kind: "add-to-attribute", - params: { attr: "RangeBonus", delta: 1 }, - }, - ], + { + descriptorId: "__test_trigger_color__", + primitives: [ + { + kind: "add-to-attribute", + params: { attr: "RangeBonus", delta: 1 }, + }, + ], + }, ]); // Make ONE white move. Black's turn now begins. The hook is on a @@ -137,7 +143,12 @@ describe("on-capture triggers", () => { // white pawn that ends up doing the capture. const whitePawn = findPiece(engine, 12); // e2 engine.session.insert(whitePawn, "OnCaptureHooks", [ - [{ kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 5 } }], + { + descriptorId: "__test_on_capture_test__", + primitives: [ + { kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 5 } }, + ], + }, ]); // White e2-e4 @@ -305,7 +316,12 @@ describe("fireOnMoveHooks", () => { const whitePawn = findPiece(engine, 12); // e2 const whiteKnight = findPiece(engine, 6); // g1 engine.session.insert(whitePawn, "OnMoveHooks", [ - [{ kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 2 } }], + { + descriptorId: "__test_on_move_fires__", + primitives: [ + { kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 2 } }, + ], + }, ]); // Knight has no hook seeded — verifies "no-attr ⇒ no-op". @@ -323,7 +339,12 @@ describe("fireOnMoveHooks", () => { const blackPawn = findPiece(engine, 52); // e7 — not moved engine.session.insert(blackPawn, "OnMoveHooks", [ - [{ kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 9 } }], + { + descriptorId: "__test_on_move_static__", + primitives: [ + { kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 9 } }, + ], + }, ]); fireOnMoveHooks(engine, [whitePawn]); // only the white pawn moved @@ -344,12 +365,14 @@ describe("fireOnTurnEndHooks", () => { // White queen has TWO hooks: one filtered to white, one to 'both'. engine.session.insert(whiteQueen, "OnTurnEndHooks", [ { + descriptorId: "__test_on_turn_end_color__", color: "white", primitives: [ { kind: "add-to-attribute", params: { attr: "HpBonus", delta: 1 } }, ], }, { + descriptorId: "__test_on_turn_end_color__", color: "both", primitives: [ { @@ -362,6 +385,7 @@ describe("fireOnTurnEndHooks", () => { // Black queen has a 'black'-only hook. engine.session.insert(blackQueen, "OnTurnEndHooks", [ { + descriptorId: "__test_on_turn_end_color__", color: "black", primitives: [ { kind: "add-to-attribute", params: { attr: "HpBonus", delta: 5 } }, @@ -392,7 +416,12 @@ describe("fireOnPromotionHooks", () => { const whitePawn = findPiece(engine, 12); engine.session.insert(whitePawn, "OnPromotionHooks", [ - [{ kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 4 } }], + { + descriptorId: "__test_on_promotion_event__", + primitives: [ + { kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 4 } }, + ], + }, ]); fireOnPromotionHooks(engine, whitePawn, "pawn", "queen"); @@ -428,7 +457,12 @@ describe("fireOnCheckReceivedHooks", () => { engine.session.insert(GAME_ENTITY, "Turn", "white"); engine.session.insert(whiteKing, "OnCheckReceivedHooks", [ - [{ kind: "add-to-attribute", params: { attr: "HpBonus", delta: 7 } }], + { + descriptorId: "__test_on_check_received_edge__", + primitives: [ + { kind: "add-to-attribute", params: { attr: "HpBonus", delta: 7 } }, + ], + }, ]); // Pre-move snapshot: white king NOT in check (empty attacker list). @@ -476,7 +510,12 @@ describe("fireOnCheckDeliveredHooks", () => { engine.session.insert(GAME_ENTITY, "Turn", "black"); engine.session.insert(whiteRook, "OnCheckDeliveredHooks", [ - [{ kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 3 } }], + { + descriptorId: "__test_on_check_delivered_discover__", + primitives: [ + { kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 3 } }, + ], + }, ]); const preDiscovered: PreMoveCheckStateLike = { @@ -502,7 +541,12 @@ describe("fireOnCheckDeliveredHooks", () => { engine.session.insert(GAME_ENTITY, "Turn", "black"); engine.session.insert(whiteRook, "OnCheckDeliveredHooks", [ - [{ kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 9 } }], + { + descriptorId: "__test_on_check_delivered_no_redundant__", + primitives: [ + { kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 9 } }, + ], + }, ]); // Already attacking pre-move — so this is NOT an edge transition @@ -528,6 +572,7 @@ describe("fireOnMovedOntoSquareHooks", () => { engine.session.insert(whitePawn, "OnMovedOntoSquareHooks", [ { + descriptorId: "__test_on_moved_onto_square_list__", filter: { kind: "squares", squares: [28, 35] }, primitives: [ { kind: "add-to-attribute", params: { attr: "HpBonus", delta: 4 } }, @@ -552,6 +597,7 @@ describe("fireOnMovedOntoSquareHooks", () => { engine.session.insert(whitePawn, "OnMovedOntoSquareHooks", [ { + descriptorId: "__test_on_moved_onto_square_rank__", filter: { kind: "predicate", rank: 3 }, // rank 4 (1-indexed) = rank 3 (0-indexed) primitives: [ { kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 1 } }, @@ -577,6 +623,7 @@ describe("fireOnMovedOntoSquareHooks", () => { engine.session.insert(whitePawn, "OnMovedOntoSquareHooks", [ { + descriptorId: "__test_on_moved_onto_square_file_rank__", filter: { kind: "predicate", file: 4, rank: 3 }, // e4 only primitives: [ { kind: "add-to-attribute", params: { attr: "HpBonus", delta: 5 } }, @@ -607,6 +654,7 @@ describe("fireOnCapturedHooks", () => { // ATTACKER (white pawn). engine.session.insert(blackPawn, "OnCapturedHooks", [ { + descriptorId: "__test_on_captured_attacker__", target: "attacker", primitives: [ { @@ -633,6 +681,7 @@ describe("fireOnCapturedHooks", () => { engine.session.insert(blackPawn, "OnCapturedHooks", [ { + descriptorId: "__test_on_captured_self__", target: "self", primitives: [ { kind: "add-to-attribute", params: { attr: "HpBonus", delta: 9 } }, @@ -957,9 +1006,12 @@ describe("deferred queue + cascade depth (T15)", () => { // primitive (`add-to-attribute`) so no further synthetic kinds // are needed. engine.session.insert(pieceId, "OnMoveHooks", [ - [ - { kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 1 } }, - ], + { + descriptorId: "__test_t15_enqueued_fires_after_arm__", + primitives: [ + { kind: "add-to-attribute", params: { attr: "RangeBonus", delta: 1 } }, + ], + }, ]); // Two-node arm: enqueuer first, marker second. The marker MUST @@ -1008,12 +1060,15 @@ describe("deferred queue + cascade depth (T15)", () => { // then drains 1..9 = 9 cascades; the 10th re-entry hits depth 9 // which triggers the > 8 check). engine.session.insert(pieceId, "OnMoveHooks", [ - [ - { - kind: "__t15_reenqueuer__" as unknown as EffectPrimitive["kind"], - params: {}, - }, - ], + { + descriptorId: "__test_t15_cascade_depth__", + primitives: [ + { + kind: "__t15_reenqueuer__" as unknown as EffectPrimitive["kind"], + params: {}, + }, + ], + }, ]); const nodes = [ @@ -1039,9 +1094,12 @@ describe("deferred queue + cascade depth (T15)", () => { // start at cascadeDepth=0; if the depth leaked, we'd accumulate // toward the 8-limit and throw on call 9 or 10. engine.session.insert(pieceId, "OnMoveHooks", [ - [ - { kind: "add-to-attribute", params: { attr: "HpBonus", delta: 1 } }, - ], + { + descriptorId: "__test_t15_no_leak__", + primitives: [ + { kind: "add-to-attribute", params: { attr: "HpBonus", delta: 1 } }, + ], + }, ]); const nodes = [ diff --git a/packages/chess/src/modifiers/triggers.ts b/packages/chess/src/modifiers/triggers.ts index 1770003..aeb662e 100644 --- a/packages/chess/src/modifiers/triggers.ts +++ b/packages/chess/src/modifiers/triggers.ts @@ -175,6 +175,26 @@ export function runPrimitives( * descriptor tree at resume time. */ triggerPath: readonly number[] = [], + /** + * T80 (Wave 14, Gap G) — id of the descriptor that OWNS this + * trigger arm. Threaded into the synthesised + * `PrimitiveApplyContext` so request-choice primitives running + * inside a trigger arm push a PendingChoice carrying the REAL + * descriptor id (not the synthetic `"__trigger__"` placeholder + * used pre-T80). `submitChoiceAndResume` resolves the descriptor + * by id and walks back into its primitive tree to continue + * execution; with the real id present, the lookup succeeds + * instead of throwing `runtime.descriptor-not-found`. + * + * Trigger dispatchers (every `fire*Hooks` below) read the id + * from the per-hook entry (`hook.descriptorId`) and pass it + * here — that is the canonical production path. Callers that + * lack a descriptor (legacy direct-fire paths in older tests, + * or any future synthetic dispatch) may omit this parameter; + * the fallback `"__trigger__"` preserves the pre-T80 behaviour + * for those code paths so the type contract remains satisfied. + */ + descriptorId: string = "__trigger__", ): void { if (depth > 8) return; // hard runtime cap, mirrors validator // T15: cascade-depth guard. Distinct from `depth` (nested primitive @@ -219,9 +239,12 @@ export function runPrimitives( session: engine.session, pieceId, depth, - // Trigger evaluation has no parent descriptor — synthesise a - // minimal ref so the type contract is satisfied. - descriptor: { id: "__trigger__", type: "data", version: 1 }, + // T80 (Wave 14, Gap G) — thread the REAL descriptor id when + // available so request-choice primitives running inside a + // trigger arm push a PendingChoice with a resolvable id. + // Pre-T80 we synthesised `"__trigger__"`; that placeholder is + // now only used by legacy callers that omit `descriptorId`. + descriptor: { id: descriptorId, type: "data", version: 1 }, // Default target stays 'self'. Hook entries that store their own // `target` (on-captured) resolve it BEFORE invoking // `runPrimitives`, calling once per resolved entity with that @@ -309,6 +332,20 @@ export function runPrimitives( } if (primitive.childPrimitives === undefined) continue; + // T82 (Gap I) — primitives that handle their own nested-list + // traversal (iteration / RNG-binding / conditional-branching + // primitives) MUST NOT have the dispatcher auto-recurse into + // their `childPrimitives()` output. apply() already walked + // `then` with the correctly extended bindings; an auto-recurse + // pass would re-execute every child a second time with the + // OUTER scope, throwing BindingError (the iteration's `$var` + // is unbound at outer scope) and double-firing imperatives. + // + // `childPrimitives()` is still defined on these primitives + // because manifest / cleanup / validator walkers need it to + // discover descendant seeds; only this RUN-time auto-recurse + // is gated by the flag. + if (primitive.selfRecurse === true) continue; let children: readonly EffectPrimitiveNode[] = []; try { // childPrimitives() introspects the ORIGINAL params — the @@ -344,6 +381,7 @@ export function runPrimitives( cascadeDepth, suppressTriggers, [...triggerPath, i], + descriptorId, ); } } @@ -502,8 +540,19 @@ export function fireOnTurnStartHooks( | ChessAttrMap["OnTurnStartHooks"] | undefined; if (hooks === undefined) continue; - for (const primitives of hooks) { - runPrimitives(engine, id, primitives, 1, undefined, new Map(), cascadeDepth); + for (const hook of hooks) { + runPrimitives( + engine, + id, + hook.primitives, + 1, + undefined, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } } @@ -529,7 +578,18 @@ export function fireOnTurnEndHooks( if (hooks === undefined) continue; for (const hook of hooks) { if (hook.color !== "both" && hook.color !== endedColor) continue; - runPrimitives(engine, id, hook.primitives, 1, undefined, new Map(), cascadeDepth); + runPrimitives( + engine, + id, + hook.primitives, + 1, + undefined, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } } @@ -553,8 +613,19 @@ export function fireOnCaptureHooks( | ChessAttrMap["OnCaptureHooks"] | undefined; if (hooks === undefined) return; - for (const primitives of hooks) { - runPrimitives(engine, attackerId, primitives, 1, undefined, new Map(), cascadeDepth); + for (const hook of hooks) { + runPrimitives( + engine, + attackerId, + hook.primitives, + 1, + undefined, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } @@ -593,8 +664,19 @@ export function fireOnDamagedHooks( | ChessAttrMap["OnDamagedHooks"] | undefined; if (hooks === undefined) continue; - for (const primitives of hooks) { - runPrimitives(engine, id, primitives, 1, undefined, new Map(), cascadeDepth); + for (const hook of hooks) { + runPrimitives( + engine, + id, + hook.primitives, + 1, + undefined, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } } @@ -643,8 +725,19 @@ export function fireOnMoveHooks( | ChessAttrMap["OnMoveHooks"] | undefined; if (hooks === undefined) continue; - for (const primitives of hooks) { - runPrimitives(engine, id, primitives, 1, undefined, new Map(), cascadeDepth); + for (const hook of hooks) { + runPrimitives( + engine, + id, + hook.primitives, + 1, + undefined, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } } @@ -674,8 +767,19 @@ export function fireOnPromotionHooks( promotedFrom, promotedTo, }; - for (const primitives of hooks) { - runPrimitives(engine, promotedPieceId, primitives, 1, event, new Map(), cascadeDepth); + for (const hook of hooks) { + runPrimitives( + engine, + promotedPieceId, + hook.primitives, + 1, + event, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } @@ -709,8 +813,19 @@ export function fireOnCheckReceivedHooks( | ChessAttrMap["OnCheckReceivedHooks"] | undefined; if (hooks === undefined) continue; - for (const primitives of hooks) { - runPrimitives(engine, royalId, primitives, 1, undefined, new Map(), cascadeDepth); + for (const hook of hooks) { + runPrimitives( + engine, + royalId, + hook.primitives, + 1, + undefined, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } } @@ -741,8 +856,19 @@ export function fireOnCheckDeliveredHooks( "OnCheckDeliveredHooks", ) as ChessAttrMap["OnCheckDeliveredHooks"] | undefined; if (hooks === undefined) continue; - for (const primitives of hooks) { - runPrimitives(engine, attackerId, primitives, 1, undefined, new Map(), cascadeDepth); + for (const hook of hooks) { + runPrimitives( + engine, + attackerId, + hook.primitives, + 1, + undefined, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } } @@ -768,7 +894,18 @@ export function fireOnMovedOntoSquareHooks( if (hooks === undefined) return; for (const hook of hooks) { if (!squareMatchesFilter(destSquare, hook.filter)) continue; - runPrimitives(engine, movedPieceId, hook.primitives, 1, undefined, new Map(), cascadeDepth); + runPrimitives( + engine, + movedPieceId, + hook.primitives, + 1, + undefined, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } @@ -852,6 +989,8 @@ export function fireOnPieceEnteredMarkerHooks( new Map(), cascadeDepth, false, + [], + hook.descriptorId, ); } } @@ -898,7 +1037,14 @@ export function fireOnCapturedHooks( session: engine.session, pieceId: capturedPieceId, depth: 0, - descriptor: { id: "__trigger__", type: "data", version: 1 }, + // T80: thread the REAL descriptor id (the descriptor that + // seeded THIS on-captured hook) through the resolver context + // and into `runPrimitives` below. Pre-T80 used the synthetic + // `"__trigger__"`; that placeholder broke + // `submitChoiceAndResume` because PendingChoices pushed inside + // a trigger arm carried the placeholder id and the descriptor + // lookup at resume time failed (Gap G). + descriptor: { id: hook.descriptorId, type: "data", version: 1 }, target: hook.target, event, // T11: empty bindings — on-captured hook entry has no enclosing @@ -917,7 +1063,18 @@ export function fireOnCapturedHooks( }; const targets = resolveTargets(resolverCtx, hook.target); for (const targetId of targets) { - runPrimitives(engine, targetId, hook.primitives, 1, event, new Map(), cascadeDepth); + runPrimitives( + engine, + targetId, + hook.primitives, + 1, + event, + new Map(), + cascadeDepth, + false, + [], + hook.descriptorId, + ); } } } @@ -972,6 +1129,9 @@ export function fireOnRuleActivatedHooks( event, new Map(), cascadeDepth, + false, + [], + hook.descriptorId, ); } } @@ -1053,6 +1213,9 @@ export function fireOnRuleExpireHooks( event, new Map(), cascadeDepth, + false, + [], + hook.descriptorId, ); } } @@ -1122,6 +1285,8 @@ export function fireOnMarkerExpireHooks( new Map(), cascadeDepth, false, + [], + hook.descriptorId, ); } } diff --git a/packages/chess/src/schema.ts b/packages/chess/src/schema.ts index 89f2250..34fedf5 100644 --- a/packages/chess/src/schema.ts +++ b/packages/chess/src/schema.ts @@ -183,16 +183,43 @@ export interface ChessAttrMap { * `computeAuraFacts` — never written directly by primitives. */ AuraContributions: Readonly>; - OnTurnStartHooks: readonly EffectPrimitiveNode[][]; - OnCaptureHooks: readonly EffectPrimitiveNode[][]; - OnDamagedHooks: readonly EffectPrimitiveNode[][]; + /** + * T80 (Wave 14, Gap G fix) — every per-piece trigger entry carries + * `descriptorId` (the id of the descriptor that seeded the hook) + * alongside the inner primitive list. Threaded through + * `runPrimitives` by the trigger dispatcher so request-choice + * primitives running inside a trigger arm push a PendingChoice + * with the REAL descriptor id (instead of the synthetic + * `"__trigger__"` placeholder used pre-T80). This lets + * `submitChoiceAndResume` resolve the descriptor by id and walk + * back into the descriptor's primitive tree to continue execution. + * + * Mirrors the existing pattern on `OnRuleActivatedHooks` / + * `OnRuleExpireHooks` / `OnPieceEnteredMarkerHooks` / + * `OnMarkerExpireHooks`, which have always carried `descriptorId`. + */ + OnTurnStartHooks: readonly { + readonly descriptorId: string; + readonly primitives: readonly EffectPrimitiveNode[]; + }[]; + OnCaptureHooks: readonly { + readonly descriptorId: string; + readonly primitives: readonly EffectPrimitiveNode[]; + }[]; + OnDamagedHooks: readonly { + readonly descriptorId: string; + readonly primitives: readonly EffectPrimitiveNode[]; + }[]; ConditionalHooks: readonly { readonly condition: ConditionSpec; readonly then: readonly EffectPrimitiveNode[]; readonly else?: readonly EffectPrimitiveNode[]; }[]; // T3-extension trigger hook attrs (read by triggers.ts evaluators added in T12) - OnMoveHooks: readonly EffectPrimitiveNode[][]; + OnMoveHooks: readonly { + readonly descriptorId: string; + readonly primitives: readonly EffectPrimitiveNode[]; + }[]; /** * On-turn-end hooks carry their `color` filter alongside the inner * primitives, because the filter must be evaluated at fire-time @@ -200,17 +227,29 @@ export interface ChessAttrMap { * against this entry to decide whether to run the inner list). */ OnTurnEndHooks: readonly { + readonly descriptorId: string; readonly color: "white" | "black" | "both"; readonly primitives: readonly EffectPrimitiveNode[]; }[]; - OnPromotionHooks: readonly EffectPrimitiveNode[][]; - OnCheckReceivedHooks: readonly EffectPrimitiveNode[][]; - OnCheckDeliveredHooks: readonly EffectPrimitiveNode[][]; + OnPromotionHooks: readonly { + readonly descriptorId: string; + readonly primitives: readonly EffectPrimitiveNode[]; + }[]; + OnCheckReceivedHooks: readonly { + readonly descriptorId: string; + readonly primitives: readonly EffectPrimitiveNode[]; + }[]; + OnCheckDeliveredHooks: readonly { + readonly descriptorId: string; + readonly primitives: readonly EffectPrimitiveNode[]; + }[]; OnCapturedHooks: readonly { + readonly descriptorId: string; readonly target: TargetResolver; readonly primitives: readonly EffectPrimitiveNode[]; }[]; OnMovedOntoSquareHooks: readonly { + readonly descriptorId: string; readonly filter: SquareFilter; readonly primitives: readonly EffectPrimitiveNode[]; }[]; diff --git a/packages/chess/src/util/pending-choices-resume-trigger.test.ts b/packages/chess/src/util/pending-choices-resume-trigger.test.ts new file mode 100644 index 0000000..1127284 --- /dev/null +++ b/packages/chess/src/util/pending-choices-resume-trigger.test.ts @@ -0,0 +1,256 @@ +/** + * T80 (Wave 14, Gap G fix) — `submitChoiceAndResume` works on + * trigger-fired PendingChoice frames. + * + * Pre-T80, when a `request-choice` ran inside an `on-captured` + * (or any other) trigger arm, the dispatcher synthesised a fake + * `descriptor: { id: "__trigger__", … }` for the + * `PrimitiveApplyContext`. The request-choice primitive read + * `ctx.descriptor.id` while pushing its `PendingChoice` frame, so + * the frame's `descriptorId` was the literal string + * `"__trigger__"`. When the player submitted the choice, + * `submitChoiceAndResume` looked up + * `engine.customModifiers.get("__trigger__")` → undefined → threw + * `runtime.descriptor-not-found`. The resume could never proceed. + * + * T80 threads the REAL descriptor id through the trigger + * dispatcher. Each `On*Hooks` entry stores the id of the + * descriptor that seeded it; every `fire*Hooks` reads the entry + * and passes the id through `runPrimitives` into the synthesised + * apply-context. The PendingChoice frame now carries the real id, + * `submitChoiceAndResume` resolves the descriptor, and the resume + * cascade runs the continuation. + * + * This test pins the contract via a parry-shaped descriptor — an + * on-captured arm wrapping a request-choice whose `then` runs an + * observable seed-attribute write. After resume, the seeded value + * is present (proving the continuation ran), and the + * `PendingChoices` stack is empty (proving the frame popped + * cleanly). + * + * Note on `cancel-capture`: the production parry descriptor uses + * `cancel-capture` in the `then` continuation, but + * `cancel-capture` requires `ctx.event.kind === "capture"` and + * `submitChoiceAndResume` passes `event: undefined` to the + * resumed `runPrimitives`. The full parry-real fixture uses a + * custom `resumeWithCaptureEvent` helper to thread the event + * through; that's outside the scope of THIS test, which focuses on + * the descriptor-id threading. Once a future task wires event + * propagation into `submitChoiceAndResume`, an additional test in + * this file can swap the seed-attribute continuation for + * cancel-capture and assert `CaptureCancelled = true`. + * + * The test uses `fireOnCapturedHooks` directly rather than driving + * the full `engine.applyMove` capture pipeline so the focus stays + * on the descriptor-id threading + resume contract — neither the + * snapshot/restore writers nor the integration-preset wiring are + * relevant here, and bypassing them keeps the test minimal. + */ +import { describe, expect, it } from "vitest"; +import type { EntityId } from "@paratype/rete"; +import { ChessEngine } from "../engine.js"; +import { + GAME_ENTITY, + type ChessAttrMap, + type PendingChoice, +} from "../schema.js"; +import { + asCustomModifierId, + type CustomModifierDescriptor, +} from "../modifiers/custom/types.js"; +import type { EffectPrimitiveNode } from "../modifiers/primitives/types.js"; +import { fireOnCapturedHooks } from "../modifiers/triggers.js"; +import { + peekPendingChoice, + submitChoiceAndResume, +} from "./pending-choices.js"; +import "../modifiers/primitives/index.js"; + +/** + * The "lifted" inner-arm shape — what the dispatcher iterates when + * firing the on-captured hook: + * + * request-choice (kind: rps, bind: defenderThrow) + * └── then: + * └── seed-attribute (HpBonus = 99 on GAME_ENTITY) + * + * In production, the parry descriptor's top-level node is + * `on-captured` and the request-choice lives in its + * `params.primitives`. The `OnCapturedHooks` hook entry stores + * exactly the INNER ARM (the request-choice). Pre-T46 the resume + * mechanism walked from the descriptor's top-level via + * `triggerPath`; with the V1 contract that path starts at the + * lifted inner arm, the simplest test scaffold registers the + * inner arm AS the descriptor's top-level primitives. This + * matches the lifting trick `mr_freeze.test.ts` / + * `mind_control.test.ts` use, and the resume mechanism walks from + * `triggerPath: []` straight into the request-choice. + * + * `seed-attribute` is used as a side-effect probe rather than + * the production `cancel-capture` — the latter requires + * `ctx.event.kind === "capture"` and `submitChoiceAndResume` + * passes `event: undefined` to the resumed `runPrimitives` (see + * file docstring). + */ +function liftedInnerArm(): readonly EffectPrimitiveNode[] { + return [ + { + kind: "request-choice", + params: { + kind: "rps", + prompt: "Defender, throw rps", + forPlayer: "both", + bind: "defenderThrow", + then: [ + { + kind: "seed-attribute", + params: { attr: "HpBonus", value: 99 }, + }, + ], + }, + }, + ]; +} + +function makeParryDescriptor(id: string): CustomModifierDescriptor { + return { + type: "data", + id: asCustomModifierId(id), + name: id, + description: "T80 Gap G test descriptor (inner arm lifted to top level)", + version: 1, + // Lifted: top-level primitives ARE the inner arm. The resume + // mechanism's `walkTriggerPath` starts here for `triggerPath: []`. + primitives: liftedInnerArm(), + targetAttrs: [], + uiForm: "primitive-composer", + source: "custom", + }; +} + +/** + * Spawn a single piece (id auto-allocated) with minimal facts so + * `fireOnCapturedHooks` can read its hook list. The defender needs + * `Position`, `PieceType`, and `Color` so resolveTargets / the + * inner arm can read it; the attacker needs the same so the event + * payload's attackerId resolves to a real entity. + */ +function spawnPiece( + engine: ChessEngine, + pieceType: "pawn" | "knight", + color: "white" | "black", + square: number, +): EntityId { + const id = engine.session.nextId(); + engine.session.insert(id, "PieceType", pieceType); + engine.session.insert(id, "Color", color); + engine.session.insert(id, "Position", square); + engine.session.insert(id, "EntityKind", "piece"); + return id; +} + +describe("T80 (Gap G) — request-choice inside on-captured can be resumed", () => { + it("fireOnCapturedHooks pushes a PendingChoice with the REAL descriptor id", () => { + const engine = new ChessEngine(); + const desc = makeParryDescriptor("test:gap-g-real-id"); + engine.customModifiers.register(desc); + + const defender = spawnPiece(engine, "pawn", "black", 35); // d5 + const attacker = spawnPiece(engine, "pawn", "white", 28); // e4 + + // Seed the on-captured hook entry with the lifted inner arm + // (= desc.primitives) and the descriptor's REAL id. Mirrors + // the apply-time shape: hook entry stores the arm, not the + // wrapping on-captured node. + engine.session.insert(defender, "OnCapturedHooks", [ + { + descriptorId: desc.id, + target: "self", + primitives: desc.primitives, + }, + ] as ChessAttrMap["OnCapturedHooks"]); + + fireOnCapturedHooks(engine, defender, attacker); + + const top = peekPendingChoice(engine); + expect(top).toBeDefined(); + // The fix: the frame carries the REAL descriptor id, not + // the synthetic "__trigger__" placeholder. + expect(top!.descriptorId).toBe(desc.id); + expect(top!.kind).toBe("rps"); + expect(top!.forPlayer).toBe("both"); + // The request-choice is the first (and only) primitive in + // the on-captured arm — index 0, no nested triggerPath. + expect(top!.primitiveIndex).toBe(0); + expect(top!.triggerPath).toEqual([]); + }); + + it("submitChoiceAndResume succeeds — descriptor lookup resolves, continuation runs", () => { + const engine = new ChessEngine(); + const desc = makeParryDescriptor("test:gap-g-resume"); + engine.customModifiers.register(desc); + + const defender = spawnPiece(engine, "pawn", "black", 35); + const attacker = spawnPiece(engine, "pawn", "white", 28); + + engine.session.insert(defender, "OnCapturedHooks", [ + { + descriptorId: desc.id, + target: "self", + primitives: desc.primitives, + }, + ] as ChessAttrMap["OnCapturedHooks"]); + + fireOnCapturedHooks(engine, defender, attacker); + + const top = peekPendingChoice(engine); + expect(top).toBeDefined(); + + // Pre-resume: the seed-attribute continuation has NOT yet run. + expect(engine.session.get(GAME_ENTITY, "HpBonus")).toBeUndefined(); + + // Pre-T80, this would throw `runtime.descriptor-not-found` + // because the frame's descriptorId was the synthetic + // "__trigger__" placeholder. T80 threads the real id, so the + // descriptor lookup succeeds and the continuation runs. + expect(() => + submitChoiceAndResume(engine, top!.choiceId, "rock"), + ).not.toThrow(); + + expect(engine.session.get(GAME_ENTITY, "HpBonus")).toBe(99); + + // Stack is empty — the resume popped the frame, the + // continuation didn't push another one. + expect( + engine.session.get(GAME_ENTITY, "PendingChoices"), + ).toEqual([] as readonly PendingChoice[]); + }); + + it("pre-T80 placeholder is gone — descriptorId NEVER equals '__trigger__' on a real fire path", () => { + // Regression guard: any future refactor that re-introduces the + // synthetic placeholder on a production fire path would break + // submitChoiceAndResume the same way Gap G did. Pin the absence + // of the placeholder so the regression surfaces here, not in a + // user-facing parry/RPS e2e flake. + const engine = new ChessEngine(); + const desc = makeParryDescriptor("test:gap-g-no-placeholder"); + engine.customModifiers.register(desc); + + const defender = spawnPiece(engine, "pawn", "black", 35); + const attacker = spawnPiece(engine, "pawn", "white", 28); + + engine.session.insert(defender, "OnCapturedHooks", [ + { + descriptorId: desc.id, + target: "self", + primitives: desc.primitives, + }, + ] as ChessAttrMap["OnCapturedHooks"]); + + fireOnCapturedHooks(engine, defender, attacker); + + const top = peekPendingChoice(engine); + expect(top).toBeDefined(); + expect(top!.descriptorId).not.toBe("__trigger__"); + }); +}); diff --git a/packages/server/src/broadcast.ts b/packages/server/src/broadcast.ts index e72000b..5320236 100644 --- a/packages/server/src/broadcast.ts +++ b/packages/server/src/broadcast.ts @@ -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 diff --git a/packages/server/src/ws.cancel-capture-revert.test.ts b/packages/server/src/ws.cancel-capture-revert.test.ts new file mode 100644 index 0000000..8893158 --- /dev/null +++ b/packages/server/src/ws.cancel-capture-revert.test.ts @@ -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 { + 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; [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 }; + 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 = { + 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): 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); + } + }); +});