diff --git a/.sisyphus/notepads/thressgame-coverage/learnings.md b/.sisyphus/notepads/thressgame-coverage/learnings.md index 71f29cf..cc99400 100644 --- a/.sisyphus/notepads/thressgame-coverage/learnings.md +++ b/.sisyphus/notepads/thressgame-coverage/learnings.md @@ -496,3 +496,398 @@ getMarkersAtSquare(square: Square): EntityId[]; // sorted by MARKER_KIND_PRIORIT - `bun test packages/chess/src/engine.spawnMarker.test.ts` → 8 pass / 0 fail / 20 expects - `bun run check` → exit 0, **2014 tests** across 171 files (was 1999 after T9; +8 from T10's new file, +7 from T7/T8 parity tests landing in same wave) - Evidence: `.sisyphus/evidence/task-10-marker-priority.txt` + + +## [2026-04-26T08:38Z] T11 binding scope stack on PrimitiveApplyContext + +### What landed +- `packages/chess/src/modifiers/primitives/context.ts` — added `BindingValue` union + `withBinding(ctx, name, value)` helper. `withBinding` clones the inner Map (`new Map(ctx.bindings)`), sets the new entry, and spreads `{...ctx, bindings: next}`. Outer ctx is NEVER mutated. +- `packages/chess/src/modifiers/primitives/types.ts` — added required field `readonly bindings: ReadonlyMap` on `PrimitiveApplyContext`. Imported `BindingValue` from `./context.js`. +- `packages/chess/src/modifiers/triggers.ts` — `runPrimitives()` gained a 6th parameter `bindings: ReadonlyMap = new Map()`. Recursive call into nested children threads the SAME map (no reset). Both context-construction sites (`runPrimitives` + `fireOnCapturedHooks`'s `resolverCtx`) seed `bindings: new Map()` / pass-through. +- `packages/chess/src/modifiers/custom/apply.ts:74` — profile-time apply seeds `bindings: new Map()`. +- `packages/chess/src/modifiers/primitives/context.test.ts` — +5 tests under `describe("binding scope (T11)")`, total 13 → 18. +- 22 test files under `packages/chess/src/modifiers/primitives/*.test.ts` updated by sed: insert `bindings: new Map(),` after `event: undefined,`. (`absorb-damage-with-attribute`, `add-aura`, `add-direction`, `add-to-attribute`, `block-move-type`, `conditional`, `modify-movement-range`, `multiply-attribute`, `on-captured`, `on-capture`, `on-check-delivered`, `on-check-received`, `on-damaged`, `on-moved-onto-square`, `on-move`, `on-promotion`, `on-turn-end`, `on-turn-start`, `override-promotion`, `reflect-damage`, `seed-attribute`, `set-capture-flag`.) + +### API contract (REUSE for T12, T13, T31-T35, T47) +```ts +export type BindingValue = + | EntityId + | readonly EntityId[] + | number // covers Square (0..63 alias) + | string + | boolean; + +export function withBinding( + ctx: PrimitiveApplyContext, + name: string, + value: BindingValue, +): PrimitiveApplyContext; // returns NEW ctx; outer untouched +``` +- **NEVER mutate `ctx.bindings` in place**. Always `withBinding(...)`. +- **`null` / `undefined` are NOT valid binding values** by design — absence means "no such binding", which keeps `ctx.bindings.get(name) === undefined` an unambiguous "not bound" sentinel for T12's `{ $var }` resolver. +- **Lexical scope**: nested primitives inherit the caller's bindings unchanged (passed through `runPrimitives`). A primitive that calls `withBinding` only affects the inner sub-tree it itself recurses into. +- **Shadowing**: rebinding the same name in an inner ctx replaces the value for that scope; the outer ctx still sees the original (immutability proof — covered by `T11.shadow` test). + +### Where to introduce bindings (downstream tasks) +- T31-T35 (iteration primitives `for-each-piece`, `for-each-square`, etc.) — call `withBinding(ctx, params.bindAs, currentItem)` per iteration, then recursively call into the nested primitive list with the new ctx. +- T37 (RNG primitive) — `withBinding(ctx, params.bindAs, rngPick)`. +- T47 (request-choice) — restored from `PendingChoice` deserialization, then `withBinding(ctx, params.bindAs, submission)` before resuming the post-choice primitive list. + +### Why required, not optional +- Making `bindings` REQUIRED on the interface (with `new Map()` at every callsite) follows the same precedent as T1's `target`/`event`. Forces dispatchers + test fixtures to think about binding scope at construction; opt-in `bindings?: ...` would lose the load-time guarantee that no path silently passes `undefined` and drops scope. +- Empty-Map default at every callsite is byte-identically backward-compatible with the 22 pre-T11 primitives — they don't read `ctx.bindings` at all. Verified: 2014 tests → 2019 tests (only +5 new tests; zero regressions). + +### Verification +- `bun test packages/chess/src/modifiers/primitives/context.test.ts` → 18 pass / 0 fail / 49 expects (was 13 / 35; +5 / +14) +- `bun run check` → exit 0, **2019 tests** across 171 files (was 2014 after T10; +5 from T11) +- LSP diagnostics: clean on `context.ts`, `types.ts`, `triggers.ts`, `context.test.ts` +- Evidence: `.sisyphus/evidence/task-11-bindings.txt` + +### Surprises / gotchas +- `PrimitiveApplyContext` is defined in `./types.js`, NOT in `./context.ts`. The `BindingValue` type lives next to `withBinding` in `context.ts` (it's a value-and-type pair); `types.ts` imports the type back via `import type { BindingValue, ... } from "./context.js"`. The import direction stays one-way (`types` ← `context` for types only) so no cycle. +- The `event,` shorthand in `runPrimitives`/`fireOnCapturedHooks` was easy to miss when grepping for `event: undefined`. Confirmed both the existing `event,` shorthand sites and added `bindings,` / `bindings: new Map()` adjacent. +- `Square` is `number` per `schema.ts` — covered by the `number` arm of `BindingValue`. No need for a separate arm. +- A callsite-counting tip for future "add a required ctx field" tasks: `grep -rn ": PrimitiveApplyContext = {" packages/chess/src/` finds every literal construction; `event: undefined,` (and `event,`) catches both default and threaded-event sites uniformly. + +## [2026-04-26T08:52Z] T14 validator: imperative-in-passive + chooser-entity stub + +### What landed +- `packages/chess/src/modifiers/custom/validate.ts`: + - New module-level `IMPERATIVE_KINDS: ReadonlySet` enumerating + the **10 LOCKED imperative kinds** (T0 ADR): `place-piece, + destroy-piece, move-piece, swap-pieces, convert-piece-type, + set-piece-attr, cancel-capture, spawn-marker, spawn-marker-pair, + destroy-marker`. Adding to or removing from this set is a + plan-amending event — Wave 5 (T21-T27) and Wave 6 (T28-T30) + register these primitives EXACTLY against the names here. + - `walkPrimitiveNodes()` extended with an `inTriggerScope: boolean` + flag threaded through recursion. Top-level invocation passes + `false` (descriptor body is passive scope). Recursion sets `true` + iff the parent kind is `"conditional"` OR matches `/^on-/`. Other + container primitives (e.g. `add-aura`) keep children in passive + scope. + - Imperative-in-passive check fires BEFORE the unknown-kind check, + so descriptors authored against the future Wave 5/6 runtime get + the precise activation-model error today (`descriptor.primitives.imperative-in-passive`). + The unknown-kind error is suppressed for IMPERATIVE_KINDS-named + nodes to avoid double-reporting. +- `packages/chess/src/modifiers/custom/apply.ts`: + - `applyCustomDescriptor` now writes the chooser color stub: + reads `Color` off the target piece, inserts + `LastModifierChooser=` on `PRESET_STATE_ENTITY` (id -1). + This is the V1 stub — when the `apply-modifier` PlayerAction + handler lands (future task), it MUST overwrite this fact with + the actual triggering player's color BEFORE invoking + `applyCustomDescriptor`. Until then, "chooser" === "owner of + target piece", which is the natural reading for self/type- + applied modifiers. +- `packages/chess/src/schema.ts`: + - Added `LastModifierChooser: PieceColor` to `ChessAttrMap`. +- `packages/chess/src/modifiers/apply.ts`: + - Added `registerAttrConsumer("LastModifierChooser")` so the + load-time integrity check sees a consumer. +- `packages/chess/src/modifiers/custom/validate.test.ts`: + - +4 new tests under `describe("imperative-in-passive + + chooser-entity (T14)")`: + 1. `destroy-piece` at top-level → REJECTED with code + `descriptor.primitives.imperative-in-passive` (and NO + `primitive.kind.unknown` double-error). + 2. `destroy-piece` inside `on-capture.params.primitives` → no + imperative-in-passive error. + 3. `spawn-marker` inside `on-turn-start → conditional → then` + → no imperative-in-passive error. + 4. `seed-attribute` carrying `value: { "ctx-attr": { entity: + "chooser", attr: "Color" } }` → validator does NOT reject + the deferred-resolution shape (T12 walker handles runtime). +- `packages/chess/src/schema.test.ts`: + - +1 round-trip test for `LastModifierChooser` on PRESET_STATE_ENTITY. + +### Trigger-scope detection rule (LOCKED for T13/T15+) +- Children of a container primitive are in trigger scope iff the + parent kind is `"conditional"` OR starts with `"on-"`. This is a + closed rule — any future trigger primitive MUST either: + (a) match the `/^on-/` naming convention, OR + (b) be added explicitly to the trigger-scope detection in + `walkPrimitiveNodes` (the `childrenInTriggerScope` derivation). +- `add-aura` and any other passive emitter keep children in passive + scope. Currently no passive emitter declares `childPrimitives`, but + the rule is set up to default-passive — the safe assumption. + +### Why imperative kinds bypass paramsSchema validation (for now) +- The 10 IMPERATIVE_KINDS aren't in `PRIMITIVE_REGISTRY` yet (Wave + 5/6 lands them). The validator early-continues when a node's kind + is in IMPERATIVE_KINDS BUT has no registry entry — skipping + paramsSchema validation + child recursion. Once Wave 5/6 lands + those primitives WITH their schemas, the validator picks them up + via the standard registry-lookup path; the early-continue becomes + unreachable for those kinds. +- The cycle / self-reference scan still runs on imperative-kind + params (it runs BEFORE the registry lookup), so structural + hazards are caught even pre-Wave-5. + +### Chooser tracking attr — name + location (FOR T12 / future apply-modifier handler) +- **Attr name**: `LastModifierChooser` (typed `PieceColor` = + `"white" | "black"`). +- **Stored on**: `PRESET_STATE_ENTITY` (id -1). +- **Written by**: `applyCustomDescriptor` in + `packages/chess/src/modifiers/custom/apply.ts` (lines ~50-55), at + the START of every descriptor application (BEFORE the primitive + walk). Read by the future T12 param walker for + `ctx-attr: { entity: "chooser", attr: "Color" }` resolution. +- **Consumer registration**: `packages/chess/src/modifiers/apply.ts` + appended right after T8's KingExtraReach. +- **Open hole** (deferred to apply-modifier action handler task): + the stub uses target piece's owner as proxy for chooser. When the + `apply-modifier` PlayerAction lands, it must write the actual + initiating player's color to `LastModifierChooser` BEFORE calling + `applyCustomDescriptor` so the stub's piece-color fallback is + superseded. + +### Subtleties / gotchas +- `seed-attribute` is the only existing primitive with `z.unknown()` + on its `value` param — it's the natural carrier for the T14 + ctx-attr-shape recognition test. Other primitives' Zod schemas + (e.g. `add-to-attribute.delta` is `z.number()`) would reject an + object value at the schema-validation stage, BUT the T12 param + walker is supposed to run BEFORE Zod validation, so that breakage + surfaces only when those schemas are exercised post-T12. T14's + test stays scoped to `seed-attribute` to avoid leaking into T12's + problem space. +- Existing `enforces max nesting depth of 3 container levels` test + uses a `destroy-piece`-free deeply-nested tree, so the new + imperative-in-passive check doesn't perturb it. Confirmed all 16 + pre-existing validate.test.ts tests still pass byte-identical. +- `inTriggerScope: false` at top-level means a passive descriptor + body that's PURELY imperative (e.g. just a `destroy-piece` at + index 0) gets ONE error per offending node, not a tree of + errors — the early-continue prevents recursion into a kind that + isn't even registered. +- The chooser stub `applyCustomDescriptor` also needs to import + `PRESET_STATE_ENTITY` and `PieceColor` from `../../schema.js` — + those weren't previously imported in `custom/apply.ts`. Added + `import { PRESET_STATE_ENTITY, type PieceColor } from "../../schema.js"`. + +### Verification +- `bun test packages/chess/src/modifiers/custom/validate.test.ts` → + 20 pass / 0 fail / 36 expects (16 existing + 4 new T14). +- `bun test packages/chess/src/schema.test.ts` → 24 pass / 0 fail + / 84 expects (23 existing + 1 new T14 chooser-attr round-trip). +- `bun run check` → **exit 0, 2024 tests across 171 files** (was + 2019 after T11; +5 from T14 = 4 validate + 1 schema). +- LSP diagnostics: clean on validate.ts, validate.test.ts, + custom/apply.ts, schema.ts, modifiers/apply.ts, schema.test.ts. +- Evidence: `.sisyphus/evidence/task-14-validator.txt` + +## [2026-04-26T09:06:29-06:00] T13 binding-scope validator + +### What landed +- `packages/chess/src/modifiers/custom/validate.ts`: + - `BINDING_INTRODUCING_KINDS: ReadonlyMap` — 8 + LOCKED entries enumerating future binder primitives + their + bind-name param key: + `for-each-piece, for-each-square, for-each-adjacent, + for-each-marker, for-column, for-row, random-pick, + request-choice` — all map to `"bind"`. + - `BINDING_CHILD_SLOTS: ReadonlySet` — 3 child-slot + names where the extended scope applies: `then, else, primitives`. + Mirrors the structural-slot convention used by `conditional` + (then/else) and trigger primitives (primitives). + - `walkBindingScope(node, inScope, errors, path)` — INDEPENDENT + second pass over the raw node tree (does NOT use + PRIMITIVE_REGISTRY child enumeration; binders aren't registered + yet). Splits a binder's params: child slots see the EXTENDED + scope (`new Set([...inScope, newName])`); non-child params + (filter, condition, count, etc.) see the OUTER scope. This + lexical-scope rule means a binder's `filter` cannot reference + its own bound name — exactly mirrors function-parameter scope. + - `checkParamsForVarRefs(value, inScope, errors, path, seen?)` — + recursive deep scan with cycle guard (the existing + `scanParamsForCyclesAndSelfReference` reports the structural + cycle separately; T13's walker just bails on `seen.has(value)`). +- `packages/chess/src/modifiers/custom/validate.test.ts`: + - +4 tests under `describe("binding-out-of-scope (T13)")`: + 1. `$var` at descriptor top → REJECTED with code + `descriptor.primitives.binding-out-of-scope` + message + containing `(none)` and `$X`. + 2. `$var` inside a synthetic `for-each-piece.then` → no + binding-out-of-scope error (other errors like unknown-kind + may still fire, that's OK). + 3. Shadowing: inner `for-each-piece` re-binds `p` from outer → + no binding-out-of-scope error inside the inner `then`. + 4. **Lexical scope guardrail**: `$p` inside the SAME binder's + `filter` (non-child slot) → REJECTED. Confirms the binder's + own non-child params see only OUTER scope. + +### Walker integration approach — SEPARATE function +- Did NOT combine with T14's `walkPrimitiveNodes`. Rationale: + `walkPrimitiveNodes` recurses via + `primitiveDescriptor.childPrimitives(...)` — a registry-driven + child-enumeration that returns `[]` for unregistered kinds. The + binders in BINDING_INTRODUCING_KINDS are ALL unregistered today + (Wave 5/7/8 lands them), so a registry-driven walker would never + descend into their `then`/`else`/`primitives` slots. T13 must + walk the raw node tree directly via the structural slot names. +- Two separate top-level invocations in `validateCustomDescriptor`: + 1. `walkPrimitiveNodes(...)` (T14 — registry-driven, threads + `inTriggerScope`) + 2. `walkBindingScope(...)` per top-level node (T13 — structural, + threads `inScopeBindings`) +- This decoupling means T13 doesn't need to coordinate with T14's + registry-traversal logic at all. Each pass owns its own concern; + errors aggregate into the same `errors` array. + +### Coordinated $var-shape detection (T12 ↔ T13 contract) +- The exact key check is `keys.length === 1 && "$var" in obj && + typeof obj.$var === "string"`. T12's runtime param-resolver MUST + use the IDENTICAL check, otherwise the validator-runtime contract + breaks (a descriptor that validates clean would still throw at + runtime, or vice versa). +- Objects like `{ $var: "X", default: 0 }` are NOT $var refs by + this check — they get walked structurally. Future + `$var-with-default` extension can be added without breaking the + current shape. + +### Subtleties +- **Cycle guard required**: T13's walker recurses into nested params + before any shape check, so the existing `t.circular` test fixture + blew the stack until I added a `seen: Set` param defaulting + to a fresh set per top-level invocation. +- **Empty top-level scope**: descriptor.primitives sees + `new Set()` — no $var refs are valid until a binder + brings a name into scope. The error message includes + `In-scope bindings: [(none)]` for top-level violations. +- **Set immutability for shadowing**: `new Set([...outer, name])` + creates a fresh set per scope; the caller's set is never + mutated. Outer scope is restored automatically when the inner + walk returns — no manual stack push/pop needed. +- **Path threading**: error paths are full + `["primitives", i, "params", "value"]`-style arrays so UI can + highlight the exact offending $var ref. + +### Verification +- `bun test packages/chess/src/modifiers/custom/validate.test.ts` + → 24 pass / 0 fail / 44 expects (was 20 / 36 pre-T13). +- `bun run check` → exit 0, **2048 tests across 172 files** (was + 2024 after T14; +24 = T13 +4 + parallel tasks landing the rest). +- LSP diagnostics: clean on validate.ts and validate.test.ts. +- Evidence: `.sisyphus/evidence/task-13-binding-scope.txt` + +## [2026-04-26T09:06:00Z] T12 param resolver + +### What landed +- **NEW** `packages/chess/src/modifiers/primitives/param-resolver.ts` (~210 lines): + - `export class BindingError extends Error` — thrown when `{ $var: name }` + references an unbound name. Carries the unbound name + lists every + binding currently in scope in the message. **T13 should import + `BindingError` from this module** for static `$var` checks (the + runtime path already throws this exact class). + - `export function resolveParams(params: unknown, ctx: PrimitiveApplyContext): unknown` + — recursive walker. Returns NEW value, never mutates. +- **NEW** `packages/chess/src/modifiers/primitives/param-resolver.test.ts` (20 tests): + - 3 no-op tests (plain primitives / objects / arrays) + - 4 `$var` tests (success, unbound BindingError, message lists `$missing`/`$a`/`$b`, `(none)` when empty) + - 7 `ctx-attr` tests (self / chooser-set / chooser-unset / chooser-no-king / numeric id / nested $var / unset attr) + - 4 `ctx-build` tests (literal e4=28, $var col+row, out-of-range throws `0..7`, non-integer) + - 2 deep-walk tests (nested resolution at arbitrary depth, multi-key plain object NOT matched) +- **MOD** `packages/chess/src/modifiers/triggers.ts` (~line 152, `runPrimitives`): + ```ts + const resolvedParams = resolveParams(node.params, ctx); + primitive.apply(ctx, resolvedParams); + ``` + Resolution happens BEFORE `primitive.apply`. `childPrimitives()` + introspection still uses the ORIGINAL unresolved params (structural + shape is independent of runtime values). +- **MOD** `packages/chess/src/modifiers/custom/apply.ts` `runPrimitive()` (~line 163): + symmetric wiring at the profile-time apply path. + +### Three resolved shapes (LOCKED API for T13/Wave 5+) + +```ts +// 1. Binding ref +{ $var: "name" } + → ctx.bindings.get("name") // throws BindingError if unbound + +// 2. Context attribute lookup +{ "ctx-attr": { entity, attr } } + → ctx.session.get(resolvedEntityId, attr) + // entity ∈ "self" | "chooser" | numeric EntityId | { $var: "..." } + // throws if attr is undefined or chooser-resolution fails + +// 3. Square computation +{ "ctx-build": { col, row } } + → col + row * 8 // Square (0..63) + // col / row may themselves be { $var } shapes + // throws if col/row out of [0..7] or non-integer +``` + +### Single-key recognition rule +A magic shape ONLY matches when the object has EXACTLY one key. So +a primitive author who legitimately stores a field literally named +`$var` alongside other fields is NEVER ambiguously rewritten. +Pinned by `does NOT match shape when the magic key is one of +multiple keys` test. + +### Chooser-entity resolution (T14 collaboration) +- `ctx-attr.entity = "chooser"` reads `LastModifierChooser` (PieceColor) + off `PRESET_STATE_ENTITY` (T14 stub stored by `applyCustomDescriptor`). +- Walker then finds king of that color, returns its EntityId; attr + lookup happens against THAT king id. +- Refinement opportunity for the apply-modifier action: store actual + chooser piece id so resolution doesn't fall back to "king of color". + +### Backward compatibility (regression-pinned) +- 22 existing primitives store ZERO objects with `$var` / `ctx-attr` / + `ctx-build` as their sole key, so the walker is a structural-clone + no-op for their params. Verified: 223/223 primitive tests pass + byte-identical (561 expects). + +### Wiring sites — definitive list +1. `packages/chess/src/modifiers/triggers.ts:152` (`runPrimitives`) +2. `packages/chess/src/modifiers/custom/apply.ts:163` (`runPrimitive`) + +Both are the SAME apply pipeline at different entry points (trigger +dispatch vs profile-time descriptor walk). Future entry points +(e.g. T47 request-choice resume) MUST also call `resolveParams` +before `primitive.apply` — no central interceptor. + +### Verification +- `bun test packages/chess/src/modifiers/primitives/param-resolver.test.ts` → 20 pass / 0 fail / 38 expects +- `bun test packages/chess/src/modifiers/primitives/` → 223 pass / 0 fail / 561 expects (regression intact) +- `bun run check` → **exit 0, 2048 tests across 172 files** (was 2024 after T14; +24) +- LSP diagnostics: clean on param-resolver.ts, param-resolver.test.ts, triggers.ts, custom/apply.ts +- Evidence: `.sisyphus/evidence/task-12-param-resolver.txt` + +### Subtleties / gotchas +- `runPrimitives` calls `primitive.apply(ctx, resolvedParams)` but + `primitive.childPrimitives(node.params)` (ORIGINAL params) — because + `childPrimitives` introspects the structural shape, and children + resolve their own params on recursion (when iteration bindings are + in scope). +- `BindingError` message format: `Binding '$NAME' is not in scope. + Available bindings: $a, $b.` (or `(none)` when empty). T13 should + produce the same shape so users see consistent messages whether + the failure is caught at validation or runtime. +- `walk()` recurses into resolver-shape values: a `$var` that resolves + to an object containing another `$var` IS walked again. Documented + (not a bug). Iteration primitives bind primitive types (EntityId / + Square / readonly EntityId[] / string / boolean), so the recursion + is a no-op in practice. +- Test fixtures: `Session.nextId()` must be called before hard-coding + `id=2` — mirrors the pattern in `seed-attribute.test.ts`. + +### Hand-off notes for T13 / downstream +- **T13** (static var-ref validator): import `BindingError` from + this module if it wants to throw the same class for static + unbound-ref errors. Validator's static analysis can borrow the + shape-detection rules verbatim (single-key match for `$var`, + `ctx-attr`, `ctx-build`). +- **Wave 5+ imperative primitives**: square selectors / entity refs + use `ctx-build` / `ctx-attr` shapes. Walker resolves before the + imperative primitive's `apply` sees them — primitives can assume + params are plain values. +- **T36/T37 (RNG)**: `bindAs` outputs flow into `ctx.bindings`; + downstream primitives read via `{ $var: bindAs }` through this + walker. +- **T47 (request-choice)**: bindings restored from `PendingChoice` + deserialisation are visible to subsequent primitives via standard + `$var` lookup. diff --git a/.sisyphus/plans/thressgame-coverage.md b/.sisyphus/plans/thressgame-coverage.md index 2827d20..5f65483 100644 --- a/.sisyphus/plans/thressgame-coverage.md +++ b/.sisyphus/plans/thressgame-coverage.md @@ -775,7 +775,7 @@ Max Concurrent: 8 (Waves 5+6+7+9 overlap) - Message: `feat(chess): marker entity factory + priority resolver` - Files: `packages/chess/src/engine.ts`, `packages/chess/src/engine.test.ts` -- [ ] 11. Binding scope stack on PrimitiveApplyContext +- [x] 11. Binding scope stack on PrimitiveApplyContext **What to do**: - Edit `packages/chess/src/modifiers/primitives/context.ts`: extend `PrimitiveApplyContext` with `bindings: ReadonlyMap` @@ -817,7 +817,7 @@ Max Concurrent: 8 (Waves 5+6+7+9 overlap) - Message: `feat(chess): binding scope on PrimitiveApplyContext` - Files: `packages/chess/src/modifiers/primitives/context.ts`, `context.test.ts` -- [ ] 12. Param walker resolves {$var}, {ctx-attr}, {ctx-build} shapes +- [x] 12. Param walker resolves {$var}, {ctx-attr}, {ctx-build} shapes **What to do**: - Create `packages/chess/src/modifiers/primitives/param-resolver.ts` exporting `resolveParams(params: unknown, ctx: PrimitiveApplyContext): unknown` — recursively walks params, substituting: @@ -861,7 +861,7 @@ Max Concurrent: 8 (Waves 5+6+7+9 overlap) - Message: `feat(chess): param walker for binding/ctx-attr/ctx-build resolution` - Files: `packages/chess/src/modifiers/primitives/param-resolver.{ts,test.ts}`, `packages/chess/src/modifiers/triggers.ts` -- [ ] 13. Validator: binding-ref-out-of-scope error +- [x] 13. Validator: binding-ref-out-of-scope error **What to do**: - Edit `packages/chess/src/modifiers/custom/validate.ts`: add a binding-scope walker that builds a binding-name set per primitive subtree and rejects any `{ $var: "X" }` reference where `X` not in scope @@ -893,7 +893,7 @@ Max Concurrent: 8 (Waves 5+6+7+9 overlap) - Message: `feat(validator): binding-scope check` - Files: `validate.ts`, `validate.test.ts` -- [ ] 14. Validator: imperative-in-passive + chooser-entity activation context +- [x] 14. Validator: imperative-in-passive + chooser-entity activation context **What to do**: - Edit validate.ts: walk descriptor tree; if a primitive whose `kind` is in IMPERATIVE_KINDS set (place-piece, destroy-piece, move-piece, swap-pieces, convert-piece-type, set-piece-attr, cancel-capture, spawn-marker, spawn-marker-pair, destroy-marker) appears OUTSIDE a trigger's `primitives` or `then`/`else` array → reject with error code `descriptor.primitives.imperative-in-passive` diff --git a/packages/chess/src/modifiers/apply.ts b/packages/chess/src/modifiers/apply.ts index 099fd9e..6f8e32c 100644 --- a/packages/chess/src/modifiers/apply.ts +++ b/packages/chess/src/modifiers/apply.ts @@ -135,6 +135,11 @@ registerAttrConsumer("MovesAlsoAs"); registerAttrConsumer("SlideMustBeMaxDistance"); registerAttrConsumer("BlockAllExceptKing"); registerAttrConsumer("KingExtraReach"); +// T14 — chooser tracking. `applyCustomDescriptor` writes +// LastModifierChooser to PRESET_STATE_ENTITY when a descriptor +// activates so `ctx-attr: { entity: "chooser", attr: "Color" }` (T12 +// param walker) can resolve to the activating player's color. +registerAttrConsumer("LastModifierChooser"); /** * Per-engine pre-move HP snapshot, used by the on-damaged trigger diff --git a/packages/chess/src/modifiers/custom/apply.ts b/packages/chess/src/modifiers/custom/apply.ts index 5a5cf5a..92a8352 100644 --- a/packages/chess/src/modifiers/custom/apply.ts +++ b/packages/chess/src/modifiers/custom/apply.ts @@ -14,7 +14,9 @@ */ import type { EntityId, Session } from "@paratype/rete"; import type { ChessEngine } from "../../engine.js"; +import { PRESET_STATE_ENTITY, type PieceColor } from "../../schema.js"; import { PRIMITIVE_REGISTRY } from "../primitives/registry.js"; +import { resolveParams } from "../primitives/param-resolver.js"; import type { EffectPrimitive, EffectPrimitiveNode, @@ -35,6 +37,30 @@ export function applyCustomDescriptor( pieceId: EntityId, descriptor: CustomModifierDescriptor, ): void { + // T14 — chooser tracking stub. The "chooser" of a descriptor is the + // player who initiated the activation (an `apply-modifier` action, + // when that PlayerAction handler lands). Until that wiring exists, + // the chooser color is INFERRED from the target piece's `Color` + // fact at apply time — i.e. the player who owns the piece receiving + // the modifier is treated as the chooser. The fact lives on + // `PRESET_STATE_ENTITY` under attr `LastModifierChooser` so the T12 + // param walker can resolve `ctx-attr: { entity: "chooser", + // attr: "Color" }` deterministically. + // + // Limitations of the stub (documented for the future apply-modifier + // action handler): + // - Profile-time applies (game-start seeding) DO write the chooser; + // in that path the "chooser" is the piece's owner, which matches + // the natural reading for self-applied / type-applied modifiers. + // - When the apply-modifier action lands it MUST overwrite this + // fact with the actual triggering player's color BEFORE running + // `applyCustomDescriptor` — the call here is a fallback so the + // fact is always present whenever a descriptor activates. + const pieceColor = session.get(pieceId, "Color") as PieceColor | undefined; + if (pieceColor === "white" || pieceColor === "black") { + session.insert(PRESET_STATE_ENTITY, "LastModifierChooser", pieceColor); + } + walkAndApply({ engine, session, @@ -86,6 +112,10 @@ function walkAndApply(input: { // override these when invoking primitives mid-game. target: "self", event: undefined, + // T11: empty binding scope at the root of every profile apply. + // Iteration / request-choice primitives use `withBinding` to + // introduce names; the 22 pre-T11 primitives don't consult it. + bindings: new Map(), }; runPrimitive(primitive, ctx, node.params); @@ -121,11 +151,17 @@ function walkAndApply(input: { * with their generic erased; we cast the apply function's first param * to `unknown` here. Any narrowing the primitive does internally * (typically via its own paramsSchema) is the primitive's contract. + * + * T12: `resolveParams` runs FIRST, substituting `{ $var }` / + * `{ "ctx-attr" }` / `{ "ctx-build" }` shapes against the current + * binding scope + session state. The 22 pre-T12 primitives never use + * those shapes, so the walker is a no-op for them. */ function runPrimitive( primitive: EffectPrimitive, ctx: PrimitiveApplyContext, params: unknown, ): void { - primitive.apply(ctx, params); + const resolvedParams = resolveParams(params, ctx); + primitive.apply(ctx, resolvedParams); } diff --git a/packages/chess/src/modifiers/custom/validate.test.ts b/packages/chess/src/modifiers/custom/validate.test.ts index 27c19d3..44817f5 100644 --- a/packages/chess/src/modifiers/custom/validate.test.ts +++ b/packages/chess/src/modifiers/custom/validate.test.ts @@ -381,3 +381,322 @@ describe("validateCustomDescriptor", () => { } }); }); + +describe("imperative-in-passive + chooser-entity (T14)", () => { + it("imperative primitive at descriptor top-level is REJECTED", () => { + // `destroy-piece` is one of the 10 IMPERATIVE_KINDS locked at T0. + // Top-level placement (i.e. directly in `descriptor.primitives`) + // violates the activation model — imperative primitives are only + // legal inside a trigger's primitive-array slot. + const descriptor = makeDescriptor(); + Object.defineProperty(descriptor, "primitives", { + value: [ + { + kind: "destroy-piece", + params: { target: "self" }, + }, + ], + }); + + const result = validateCustomDescriptor(descriptor); + expect(result.ok).toBe(false); + if (!result.ok) { + expect( + result.errors.some( + (e) => e.code === "descriptor.primitives.imperative-in-passive", + ), + ).toBe(true); + // The error path points at the offending node, not the + // descriptor root. + const offending = result.errors.find( + (e) => e.code === "descriptor.primitives.imperative-in-passive", + ); + expect(offending?.path).toEqual(["primitives", 0]); + // Imperative kinds skip the unknown-kind error even when not + // yet registered (Wave 5/6 lands the registry entries). + expect( + result.errors.some((e) => e.code === "primitive.kind.unknown"), + ).toBe(false); + } + }); + + it("imperative primitive INSIDE on-capture's primitives array is ACCEPTED", () => { + // Same imperative kind as above, but wrapped in `on-capture` — + // its `primitives` slot IS a trigger-scope primitive-array. + const descriptor = makeDescriptor(); + Object.defineProperty(descriptor, "primitives", { + value: [ + { + kind: "on-capture", + params: { + primitives: [ + { kind: "destroy-piece", params: { target: "self" } }, + ], + }, + }, + ], + }); + + const result = validateCustomDescriptor(descriptor); + // The validator must NOT flag imperative-in-passive here; other + // errors (e.g. unknown-kind for not-yet-registered imperative + // primitives) are intentionally suppressed at T14 too. + if (!result.ok) { + expect( + result.errors.some( + (e) => e.code === "descriptor.primitives.imperative-in-passive", + ), + ).toBe(false); + expect( + result.errors.some((e) => e.code === "primitive.kind.unknown"), + ).toBe(false); + } else { + expect(result.ok).toBe(true); + } + }); + + it("imperative primitive INSIDE conditional.then is ACCEPTED", () => { + // Conditional `then` arm is also trigger-scope (it's only legal + // inside a trigger itself, but at the validator level we treat + // any conditional's then/else arm as scope-equivalent — depth + // and outer-scope checks handle the higher-level structure). + const descriptor = makeDescriptor(); + Object.defineProperty(descriptor, "primitives", { + value: [ + { + kind: "on-turn-start", + params: { + primitives: [ + { + kind: "conditional", + params: { + condition: { type: "always" }, + then: [ + { kind: "spawn-marker", params: { kind: "mine" } }, + ], + }, + }, + ], + }, + }, + ], + }); + + const result = validateCustomDescriptor(descriptor); + if (!result.ok) { + expect( + result.errors.some( + (e) => e.code === "descriptor.primitives.imperative-in-passive", + ), + ).toBe(false); + expect( + result.errors.some((e) => e.code === "primitive.kind.unknown"), + ).toBe(false); + } else { + expect(result.ok).toBe(true); + } + }); + + it("ctx-attr: { entity: 'chooser', attr: 'Color' } param shape is RECOGNIZED (no rejection)", () => { + // T12 ships the param walker that resolves these shapes at + // runtime. T14's job is just to TOLERATE the shape so descriptors + // authored against the future runtime validate today. The validator + // walks params for cycles / self-reference / Zod-schema match, and + // a `ctx-attr` object value must NOT be flagged by any of those. + // + // We use `seed-attribute` as the carrier because its `value` + // param is `z.unknown()` — accepts arbitrary deferred-resolution + // shapes. (Other primitives with `z.number()` values would + // reject the object outright; T12 wires walker BEFORE Zod + // validation, so this stays a future-task concern.) + const descriptor = makeDescriptor(); + Object.defineProperty(descriptor, "primitives", { + value: [ + { + kind: "seed-attribute", + params: { + attr: "LastModifierChooser", + value: { "ctx-attr": { entity: "chooser", attr: "Color" } }, + }, + }, + ], + }); + + const result = validateCustomDescriptor(descriptor); + // No error related to the ctx-attr shape — neither cycle, nor + // self-reference, nor any new T14 error code targets it. (Other + // pre-existing errors against this primitive would fail this + // test by changing the shape of `result.errors`; assert + // explicitly that ctx-attr isn't called out.) + if (!result.ok) { + const ctxAttrErrors = result.errors.filter((e) => + e.message.includes("ctx-attr"), + ); + expect(ctxAttrErrors).toEqual([]); + expect( + result.errors.some( + (e) => e.code === "descriptor.primitives.imperative-in-passive", + ), + ).toBe(false); + } else { + expect(result.ok).toBe(true); + } + }); +}); + +describe("binding-out-of-scope (T13)", () => { + it("$var reference at descriptor top is REJECTED", () => { + // `seed-attribute.value` is `z.unknown()` so the $var object + // shape passes paramsSchema and reaches the T13 walker. No + // binding-introducing primitive is in scope at descriptor top + // (Wave 5 hasn't landed any binder yet), so $X is unbound. + const descriptor = makeDescriptor(); + Object.defineProperty(descriptor, "primitives", { + value: [ + { + kind: "seed-attribute", + params: { attr: "HpBonus", value: { $var: "X" } }, + }, + ], + }); + + const result = validateCustomDescriptor(descriptor); + expect(result.ok).toBe(false); + if (!result.ok) { + expect( + result.errors.some( + (e) => e.code === "descriptor.primitives.binding-out-of-scope", + ), + ).toBe(true); + const oos = result.errors.find( + (e) => e.code === "descriptor.primitives.binding-out-of-scope", + ); + expect(oos?.message).toContain("$X"); + expect(oos?.message).toContain("(none)"); + } + }); + + it("$var inside binder's child arm is ACCEPTED (synthetic for-each-piece)", () => { + // `for-each-piece` is in BINDING_INTRODUCING_KINDS but isn't + // registered yet (Wave 5 lands T31). The walker still descends + // into its `then` child slot so $var resolution works today. + // Other errors (unknown-kind for the unregistered binder, + // imperative-in-passive for the inner set-piece-attr outside + // a trigger) MAY surface — we only assert binding-out-of-scope + // is NOT among them. + const descriptor = makeDescriptor(); + Object.defineProperty(descriptor, "primitives", { + value: [ + { + kind: "for-each-piece", + params: { + filter: { color: "white" }, + bind: "p", + then: [ + { + kind: "set-piece-attr", + params: { + target: { $var: "p" }, + attr: "Hp", + value: 5, + }, + }, + ], + }, + }, + ], + }); + + const result = validateCustomDescriptor(descriptor); + if (!result.ok) { + expect( + result.errors.some( + (e) => e.code === "descriptor.primitives.binding-out-of-scope", + ), + ).toBe(false); + } else { + expect(result.ok).toBe(true); + } + }); + + it("shadowing: inner re-bind of same name is allowed", () => { + // Outer for-each-piece binds `p` to white pieces; inner + // for-each-piece re-binds `p` to black pieces. The $var ref + // inside the inner `then` resolves to the SHADOWED p — no + // error. Set-add semantics: `new Set([...outer, "p"])` keeps + // `p` in scope; the inner walker's childScope is a fresh set, + // so the outer binding is not mutated. + const descriptor = makeDescriptor(); + Object.defineProperty(descriptor, "primitives", { + value: [ + { + kind: "for-each-piece", + params: { + filter: { color: "white" }, + bind: "p", + then: [ + { + kind: "for-each-piece", + params: { + filter: { color: "black" }, + bind: "p", + then: [ + { + kind: "set-piece-attr", + params: { + target: { $var: "p" }, + attr: "Hp", + value: 1, + }, + }, + ], + }, + }, + ], + }, + }, + ], + }); + + const result = validateCustomDescriptor(descriptor); + if (!result.ok) { + expect( + result.errors.some( + (e) => e.code === "descriptor.primitives.binding-out-of-scope", + ), + ).toBe(false); + } else { + expect(result.ok).toBe(true); + } + }); + + it("$var inside a binder's NON-child param (e.g. filter) is REJECTED", () => { + // The bound name only takes effect in child slots. Referencing + // `$p` inside the `filter` of the SAME for-each-piece that + // binds `p` is out-of-scope — lexical scope rule (a function + // parameter isn't visible in its own argument expression). + const descriptor = makeDescriptor(); + Object.defineProperty(descriptor, "primitives", { + value: [ + { + kind: "for-each-piece", + params: { + filter: { color: { $var: "p" } }, + bind: "p", + then: [], + }, + }, + ], + }); + + const result = validateCustomDescriptor(descriptor); + expect(result.ok).toBe(false); + if (!result.ok) { + expect( + result.errors.some( + (e) => e.code === "descriptor.primitives.binding-out-of-scope", + ), + ).toBe(true); + } + }); +}); diff --git a/packages/chess/src/modifiers/custom/validate.ts b/packages/chess/src/modifiers/custom/validate.ts index 20b955d..884cff8 100644 --- a/packages/chess/src/modifiers/custom/validate.ts +++ b/packages/chess/src/modifiers/custom/validate.ts @@ -16,6 +16,85 @@ const MAX_DESCRIPTOR_DESCRIPTION_LENGTH = 200; const MAX_RECURSION_DEPTH = 3; const MAX_PRIMITIVE_COUNT = 50; +/** + * T14 — primitive kinds that mutate board state. These are LEGAL + * ONLY inside a trigger's primitive-array slot (`on-*.params.primitives` + * or `conditional.params.then` / `conditional.params.else`, which are + * themselves only legal inside a trigger). + * + * Top-level placement of an imperative primitive (i.e. directly in + * `descriptor.primitives`) is rejected with error code + * `descriptor.primitives.imperative-in-passive`. The activation model + * (see `.sisyphus/notepads/thressgame-coverage/decisions.md` + * § Activation Model) cleanly separates "describe the world" (passive) + * from "mutate the world" (imperative inside triggers). + * + * The 10 kinds here are LOCKED at the T0 ADR. Adding to or removing + * from this set requires a plan amendment, not an in-flight tweak — + * downstream waves (5/6 — T21-T30) implement these primitives + * EXACTLY against this list. + * + * Note: these kinds are NOT yet present in `PRIMITIVE_REGISTRY` — + * Wave 5/6 will register them. The validator checks imperative-in- + * passive BEFORE the unknown-kind check so descriptors authored + * against a future runtime get a precise error code today. + */ +const IMPERATIVE_KINDS: ReadonlySet = new Set([ + "place-piece", + "destroy-piece", + "move-piece", + "swap-pieces", + "convert-piece-type", + "set-piece-attr", + "cancel-capture", + "spawn-marker", + "spawn-marker-pair", + "destroy-marker", +]); + +/** + * T13 — primitive kinds that introduce a NEW binding name into the + * lexical scope of their child arms. The map value is the param key + * holding the bound-name string (e.g. `for-each-piece` reads its + * `bind` param: `{ kind: "for-each-piece", params: { filter, bind: + * "p", then: [...] } }` introduces `p` into the scope of `then`). + * + * The 8 kinds enumerated here are LOCKED at the T0 ADR for binding + * scope. None of them are registered yet (Wave 5 lands T31-T35 + * iteration primitives, Wave 7 lands T36-T37 RNG primitives, Wave 8 + * lands T47 request-choice). Pre-populating the map now means when + * those primitives land, the scope walker requires zero edits — it + * just starts honouring the new kinds. + * + * Adding to / removing from this map is a plan-amending event: + * downstream wave tasks implement these primitives EXACTLY against + * the names + bind-keys here. + */ +const BINDING_INTRODUCING_KINDS: ReadonlyMap = new Map([ + ["for-each-piece", "bind"], + ["for-each-square", "bind"], + ["for-each-adjacent", "bind"], + ["for-each-marker", "bind"], + ["for-column", "bind"], + ["for-row", "bind"], + ["random-pick", "bind"], + ["request-choice", "bind"], +]); + +/** + * T13 — child-slot names whose contents inherit the EXTENDED binding + * scope of a binding-introducing primitive (the bind name is in scope + * inside `then`/`else`/`primitives` but NOT inside the binder's other + * params like `filter` / `condition` / `count`). Mirrors the + * structural-slot convention used by `conditional` (then/else) and + * the trigger primitives (primitives). + */ +const BINDING_CHILD_SLOTS: ReadonlySet = new Set([ + "then", + "else", + "primitives", +]); + export function validateCustomDescriptor( descriptor: CustomModifierDescriptor, ): ValidationResult { @@ -74,8 +153,28 @@ export function validateCustomDescriptor( containerDepth: 0, basePath: ["primitives"], walkState, + // T14 — top-level descriptor body is PASSIVE scope. Imperative + // primitives here are rejected; trigger primitives (`on-*`, + // `conditional`, `add-aura`, etc.) flip their children into + // trigger scope when recursed. + inTriggerScope: false, }); + // T13 — binding-scope walk. Independent pass over the raw node + // tree (does NOT rely on PRIMITIVE_REGISTRY child enumeration) + // because the binding-introducing kinds (Wave 5/6/7 — see + // BINDING_INTRODUCING_KINDS above) aren't registered yet; their + // `then` / `else` / `primitives` slots must still be visited to + // resolve `{ $var: "X" }` references at the descriptor-validation + // boundary. Top-level descriptor body has an EMPTY binding scope + // — no $var refs are valid until a binding-introducing primitive + // brings a name into scope. + for (let i = 0; i < descriptor.primitives.length; i += 1) { + const node = descriptor.primitives[i]; + if (node === undefined) continue; + walkBindingScope(node, new Set(), errors, ["primitives", i]); + } + if (errors.length === 0) { return { ok: true }; } @@ -93,8 +192,23 @@ function walkPrimitiveNodes(input: { totalPrimitiveCount: number; emittedPrimitiveCountError: boolean; }; + /** + * T14 — true when the parent slot is a trigger's primitive-array + * (`on-*.params.primitives` / `conditional.params.then` / `.else`). + * Imperative primitives (kind ∈ IMPERATIVE_KINDS) are rejected + * unless this flag is set. + */ + inTriggerScope: boolean; }): void { - const { nodes, errors, descriptorId, containerDepth, basePath, walkState } = input; + const { + nodes, + errors, + descriptorId, + containerDepth, + basePath, + walkState, + inTriggerScope, + } = input; for (let index = 0; index < nodes.length; index += 1) { const node = nodes[index]; @@ -115,6 +229,23 @@ function walkPrimitiveNodes(input: { walkState.emittedPrimitiveCountError = true; } + // T14 — imperative-in-passive check. Runs BEFORE the unknown-kind + // check so that descriptors authored against the (future) Wave 5/6 + // imperative primitives get the precise activation-model error + // even before those primitives are registered. The 10 imperative + // kinds are listed in IMPERATIVE_KINDS above; their bodies land + // in T21-T30. Until then, recognising the kind by NAME is the + // contract the validator commits to. + if (IMPERATIVE_KINDS.has(node.kind) && !inTriggerScope) { + errors.push({ + code: "descriptor.primitives.imperative-in-passive", + path: nodePath, + message: + `Imperative primitive '${node.kind}' is only legal inside a ` + + "trigger's primitives/then/else array.", + }); + } + scanParamsForCyclesAndSelfReference({ value: node.params, descriptorId, @@ -126,6 +257,14 @@ function walkPrimitiveNodes(input: { const primitiveDescriptor = PRIMITIVE_REGISTRY.get(node.kind); if (primitiveDescriptor === undefined) { + // T14 — imperative kinds are recognised by name even when not + // yet registered (Wave 5/6 will register them); skip the + // unknown-kind error so the same descriptor doesn't double- + // report. The imperative-in-passive error above already + // surfaces the misuse if any. + if (IMPERATIVE_KINDS.has(node.kind)) { + continue; + } errors.push({ code: "primitive.kind.unknown", path: [...nodePath, "kind"], @@ -167,6 +306,19 @@ function walkPrimitiveNodes(input: { children = []; } + // T14 — children of a trigger (`on-*`) or `conditional` are in + // trigger scope (their primitive-array slots are the canonical + // legal home of imperative primitives). Children of any other + // container (e.g. `add-aura` — currently has none, but future + // passive containers MUST be defaulted to passive) stay passive. + // + // Detection by kind-name keeps the rule data-driven without + // adding a new field to EffectPrimitive. The set is closed: any + // future trigger primitive must add itself here OR explicitly + // declare its scope semantics in the EffectPrimitive contract. + const childrenInTriggerScope = + node.kind === "conditional" || node.kind.startsWith("on-"); + walkPrimitiveNodes({ nodes: children, errors, @@ -174,6 +326,7 @@ function walkPrimitiveNodes(input: { containerDepth: nextDepth, basePath: [...nodePath, "children"], walkState, + inTriggerScope: childrenInTriggerScope, }); } } @@ -248,3 +401,145 @@ function scanParamsForCyclesAndSelfReference(input: { stack.delete(value); } + +/** + * T13 — recursively scan a params subtree for `{ $var: "X" }` + * reference shapes and assert each `X` is in the current binding + * scope. Mirrors the runtime resolver's exact key check (T12) so + * that any descriptor accepted by the validator survives runtime + * resolution: only objects with a SINGLE key `"$var"` whose value + * is a string count as a $var reference. Objects with additional + * keys are walked structurally (e.g. `ctx-attr` / `ctx-build` + * shapes have nested params that may contain $var refs). + */ +function checkParamsForVarRefs( + value: unknown, + inScopeBindings: ReadonlySet, + errors: ValidationError[], + path: (string | number)[], + seen: Set = new Set(), +): void { + if (value === null || typeof value !== "object") return; + + // Cycle guard — `scanParamsForCyclesAndSelfReference` reports the + // structural cycle separately; here we just bail to avoid stack + // overflow on shared / cyclic substructures. + if (seen.has(value)) return; + seen.add(value); + + if (Array.isArray(value)) { + for (let i = 0; i < value.length; i += 1) { + checkParamsForVarRefs(value[i], inScopeBindings, errors, [...path, i], seen); + } + return; + } + + const obj = value as Record; + const keys = Object.keys(obj); + + // T13 — exact $var-shape detection. Coordinated with T12's runtime + // resolver: a $var ref is `{ $var: "" }` with EXACTLY one + // key. Anything else (even `{ $var: "X", default: 0 }`) is walked + // structurally — those alternative shapes are NOT $var refs. + if ( + keys.length === 1 && + "$var" in obj && + typeof obj.$var === "string" + ) { + if (!inScopeBindings.has(obj.$var)) { + const inScopeList = Array.from(inScopeBindings).join(", ") || "(none)"; + errors.push({ + code: "descriptor.primitives.binding-out-of-scope", + path, + message: + `Binding $${obj.$var} is not in scope. ` + + `In-scope bindings: [${inScopeList}].`, + }); + } + return; + } + + for (const [key, nested] of Object.entries(obj)) { + checkParamsForVarRefs(nested, inScopeBindings, errors, [...path, key], seen); + } +} + +/** + * T13 — walk one primitive node, validating that every + * `{ $var: "X" }` reference inside its params resolves against the + * inherited binding scope, and recursing into the node's child + * slots (`then` / `else` / `primitives`) with the (possibly + * extended) scope. + * + * Binding-introducing primitives (kind ∈ BINDING_INTRODUCING_KINDS) + * extend the scope ONLY for their child slots — the binder's OWN + * params (e.g. the `filter` clause of `for-each-piece`) cannot + * reference the bound name. This mirrors lexical scope as a function + * parameter is bound only inside the function body, not in the + * function's own argument expressions. + * + * Shadowing is handled implicitly by Set's add semantics: rebinding + * the same name in an inner scope overwrites the outer entry FOR + * the inner subtree; the caller's set is untouched (defensive + * cloning via `new Set([...inScopeBindings, newName])`). + */ +function walkBindingScope( + node: EffectPrimitiveNode, + inScopeBindings: ReadonlySet, + errors: ValidationError[], + path: (string | number)[], +): void { + if (node === null || typeof node !== "object") return; + + // Identify whether this node introduces a new binding. The bind + // name is computed FIRST so we can split the params scan: the + // binder's non-child params are scanned against the OUTER scope + // (the new name isn't visible to siblings of the child slots). + const bindKey = BINDING_INTRODUCING_KINDS.get(node.kind); + let childScope: ReadonlySet = inScopeBindings; + if (bindKey !== undefined && node.params !== null && typeof node.params === "object") { + const params = node.params as Record; + const newName = params[bindKey]; + if (typeof newName === "string" && newName.length > 0) { + childScope = new Set([...inScopeBindings, newName]); + } + } + + // Scan THIS node's params for $var refs, splitting child slots + // (which see `childScope`) from the rest (which see + // `inScopeBindings`). For non-binders, both scopes are the same + // set, so the split is a no-op. + if (node.params !== null && typeof node.params === "object" && !Array.isArray(node.params)) { + const params = node.params as Record; + for (const [key, value] of Object.entries(params)) { + if (BINDING_CHILD_SLOTS.has(key)) { + // Child slot — recurse into its primitive nodes with the + // (possibly extended) childScope. Each nested primitive + // gets walkBindingScope; the params content of the child + // slot itself is the array of nodes, not arbitrary data. + if (Array.isArray(value)) { + for (let i = 0; i < value.length; i += 1) { + const child = value[i]; + if (child !== null && typeof child === "object" && "kind" in child) { + walkBindingScope( + child as EffectPrimitiveNode, + childScope, + errors, + [...path, "params", key, i], + ); + } + } + } + } else { + // Non-child param — scan with OUTER scope so the binder's + // own filter/condition/count etc. can't reference its own + // bound name (binding only takes effect inside child slots). + checkParamsForVarRefs(value, inScopeBindings, errors, [ + ...path, + "params", + key, + ]); + } + } + } +} diff --git a/packages/chess/src/modifiers/primitives/absorb-damage-with-attribute.test.ts b/packages/chess/src/modifiers/primitives/absorb-damage-with-attribute.test.ts index 6df9c40..58b1d0c 100644 --- a/packages/chess/src/modifiers/primitives/absorb-damage-with-attribute.test.ts +++ b/packages/chess/src/modifiers/primitives/absorb-damage-with-attribute.test.ts @@ -17,6 +17,7 @@ function makeContext(session: Session, pieceId: EntityId) { }, target: "self" as const, event: undefined, + bindings: new Map(), }; } diff --git a/packages/chess/src/modifiers/primitives/add-aura.test.ts b/packages/chess/src/modifiers/primitives/add-aura.test.ts index 6863be7..08cf78e 100644 --- a/packages/chess/src/modifiers/primitives/add-aura.test.ts +++ b/packages/chess/src/modifiers/primitives/add-aura.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/add-direction.test.ts b/packages/chess/src/modifiers/primitives/add-direction.test.ts index eb8e652..df1f59a 100644 --- a/packages/chess/src/modifiers/primitives/add-direction.test.ts +++ b/packages/chess/src/modifiers/primitives/add-direction.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/add-to-attribute.test.ts b/packages/chess/src/modifiers/primitives/add-to-attribute.test.ts index 4b160e7..2606e3c 100644 --- a/packages/chess/src/modifiers/primitives/add-to-attribute.test.ts +++ b/packages/chess/src/modifiers/primitives/add-to-attribute.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/block-move-type.test.ts b/packages/chess/src/modifiers/primitives/block-move-type.test.ts index 12616eb..49f0e12 100644 --- a/packages/chess/src/modifiers/primitives/block-move-type.test.ts +++ b/packages/chess/src/modifiers/primitives/block-move-type.test.ts @@ -17,6 +17,7 @@ function makeContext(session: Session, pieceId: EntityId) { }, target: "self" as const, event: undefined, + bindings: new Map(), }; } diff --git a/packages/chess/src/modifiers/primitives/conditional.test.ts b/packages/chess/src/modifiers/primitives/conditional.test.ts index e1659a2..f6b0da6 100644 --- a/packages/chess/src/modifiers/primitives/conditional.test.ts +++ b/packages/chess/src/modifiers/primitives/conditional.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/context.test.ts b/packages/chess/src/modifiers/primitives/context.test.ts index 0e69955..435271a 100644 --- a/packages/chess/src/modifiers/primitives/context.test.ts +++ b/packages/chess/src/modifiers/primitives/context.test.ts @@ -14,8 +14,12 @@ import type { EntityId } from "@paratype/rete"; import { describe, expect, it } from "vitest"; import { ChessEngine } from "../../engine.js"; import type { PieceColor, PieceType } from "../../schema.js"; -import { resolveTargets } from "./context.js"; -import type { PrimitiveEvent, TargetResolver } from "./context.js"; +import { resolveTargets, withBinding } from "./context.js"; +import type { + BindingValue, + PrimitiveEvent, + TargetResolver, +} from "./context.js"; import type { PrimitiveApplyContext } from "./types.js"; /** @@ -47,6 +51,7 @@ function buildFixture( descriptor: { id: "custom:test-context", type: "data", version: 1 }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session, ids }; } @@ -228,3 +233,102 @@ describe("PrimitiveEvent — promotion variant", () => { } }); }); + +/** + * T11 — lexically-scoped binding stack on `PrimitiveApplyContext`. + * + * `withBinding(ctx, name, value)` returns a NEW context whose + * `bindings` map contains every entry from the outer ctx plus the + * supplied name/value. The outer ctx is never mutated, supporting + * lexical-scope semantics for iteration / request-choice primitives + * to land in subsequent waves (T31-T35, T47). + */ +describe("binding scope (T11)", () => { + it("default ctx.bindings is an empty Map (backward-compat for the 22 pre-T11 primitives)", () => { + const { ctx } = buildFixture([ + { color: "white", type: "knight", square: 1 }, + ]); + expect(ctx.bindings).toBeInstanceOf(Map); + expect(ctx.bindings.size).toBe(0); + }); + + it("withBinding returns a NEW context object (immutability)", () => { + const { ctx } = buildFixture([ + { color: "white", type: "knight", square: 1 }, + ]); + const next = withBinding(ctx, "x", 42); + // Outer ctx is untouched — proves the bindings map was cloned, not + // mutated, and the surrounding ctx reference is fresh. + expect(ctx.bindings.size).toBe(0); + expect(next).not.toBe(ctx); + expect(next.bindings).not.toBe(ctx.bindings); + expect(next.bindings.size).toBe(1); + expect(next.bindings.get("x")).toBe(42); + // Other ctx fields are preserved by the spread. + expect(next.engine).toBe(ctx.engine); + expect(next.pieceId).toBe(ctx.pieceId); + expect(next.target).toBe(ctx.target); + }); + + it("3 levels of nested bindings: each level sees its own + ancestors", () => { + const { ctx: root } = buildFixture([ + { color: "white", type: "knight", square: 1 }, + ]); + const lvl1 = withBinding(root, "a", 1); + const lvl2 = withBinding(lvl1, "b", 2); + const lvl3 = withBinding(lvl2, "c", 3); + // Innermost sees every ancestor binding. + expect(lvl3.bindings.get("a")).toBe(1); + expect(lvl3.bindings.get("b")).toBe(2); + expect(lvl3.bindings.get("c")).toBe(3); + expect(lvl3.bindings.size).toBe(3); + // Mid-level sees only its own + outer. + expect(lvl2.bindings.get("a")).toBe(1); + expect(lvl2.bindings.get("b")).toBe(2); + expect(lvl2.bindings.get("c")).toBeUndefined(); + expect(lvl2.bindings.size).toBe(2); + // Root remains pristine — no leak from inner scopes. + expect(root.bindings.size).toBe(0); + }); + + it("later binding shadows earlier one of the same name (outer ctx unaffected)", () => { + const { ctx: root } = buildFixture([ + { color: "white", type: "knight", square: 1 }, + ]); + const outer = withBinding(root, "x", 1); + const inner = withBinding(outer, "x", 2); + // Inner sees the SHADOWED value. + expect(inner.bindings.get("x")).toBe(2); + expect(inner.bindings.size).toBe(1); + // Outer ctx still resolves the original value — proof the inner + // shadow did not retroactively mutate its parent's map. + expect(outer.bindings.get("x")).toBe(1); + expect(outer.bindings.size).toBe(1); + // Root is still empty. + expect(root.bindings.size).toBe(0); + }); + + it("withBinding accepts each BindingValue variant (number, string, boolean, EntityId, readonly EntityId[])", () => { + const { ctx: root, ids } = buildFixture([ + { color: "white", type: "knight", square: 1 }, + { color: "white", type: "pawn", square: 12 }, + { color: "black", type: "queen", square: 59 }, + ]); + // Sanity: assignment to BindingValue locals proves the union covers + // each shape downstream consumers will use. + const asNumber: BindingValue = 7; + const asString: BindingValue = "hello"; + const asBool: BindingValue = true; + const asEntity: BindingValue = ids[0]!; + const asArray: BindingValue = [ids[1]!, ids[2]!] as const; + + expect(withBinding(root, "n", asNumber).bindings.get("n")).toBe(7); + expect(withBinding(root, "s", asString).bindings.get("s")).toBe("hello"); + expect(withBinding(root, "b", asBool).bindings.get("b")).toBe(true); + expect(withBinding(root, "e", asEntity).bindings.get("e")).toBe(ids[0]); + expect(withBinding(root, "arr", asArray).bindings.get("arr")).toEqual([ + ids[1], + ids[2], + ]); + }); +}); diff --git a/packages/chess/src/modifiers/primitives/context.ts b/packages/chess/src/modifiers/primitives/context.ts index 6fe79ca..089b9d1 100644 --- a/packages/chess/src/modifiers/primitives/context.ts +++ b/packages/chess/src/modifiers/primitives/context.ts @@ -37,6 +37,53 @@ import type { } from "../../schema.js"; import type { PrimitiveApplyContext } from "./types.js"; +/** + * Value types that may be stored under a binding key in + * `PrimitiveApplyContext.bindings` (T11). + * + * Iteration primitives (T31-T35) bind `EntityId` / `Square` / readonly + * arrays; RNG (T37) binds picks; request-choice (T47) binds the + * player's submission. `Square` is a numeric alias (0..63) so it falls + * under the `number` arm of the union. + * + * `null` / `undefined` are intentionally NOT permitted — bindings are + * either present (with a defined value) or absent (omitted from the + * map). This keeps `ctx.bindings.get(name) === undefined` an + * unambiguous "no such binding" signal at lookup sites. + */ +export type BindingValue = + | EntityId + | readonly EntityId[] + | number + | string + | boolean; + +/** + * Returns a NEW `PrimitiveApplyContext` with one additional binding. + * The outer context's `bindings` map is left untouched — supporting + * lexical-scope semantics: nested primitives see ancestor bindings + * while later iterations of the same scope re-derive from the outer. + * + * Mutation rule: NEVER mutate `ctx.bindings` in place. Always go + * through this helper. Iteration primitives (T31-T35) and + * request-choice (T47) call this once per introduced name; the + * existing 22 primitives don't touch bindings at all and inherit the + * empty default unchanged. + * + * Shadowing: if `name` already exists in `ctx.bindings`, the returned + * context's value REPLACES the prior one for the inner scope. The + * outer ctx still resolves the older value (immutability proof). + */ +export function withBinding( + ctx: PrimitiveApplyContext, + name: string, + value: BindingValue, +): PrimitiveApplyContext { + const next = new Map(ctx.bindings); + next.set(name, value); + return { ...ctx, bindings: next }; +} + /** * Discriminated union of trigger-supplied event metadata. Each variant * is branded by `kind` so the type narrows cleanly at use-sites and so diff --git a/packages/chess/src/modifiers/primitives/modify-movement-range.test.ts b/packages/chess/src/modifiers/primitives/modify-movement-range.test.ts index e530dae..c55285b 100644 --- a/packages/chess/src/modifiers/primitives/modify-movement-range.test.ts +++ b/packages/chess/src/modifiers/primitives/modify-movement-range.test.ts @@ -17,6 +17,7 @@ function makeContext(session: Session, pieceId: EntityId) { }, target: "self" as const, event: undefined, + bindings: new Map(), }; } diff --git a/packages/chess/src/modifiers/primitives/multiply-attribute.test.ts b/packages/chess/src/modifiers/primitives/multiply-attribute.test.ts index 85211fe..6c9bf87 100644 --- a/packages/chess/src/modifiers/primitives/multiply-attribute.test.ts +++ b/packages/chess/src/modifiers/primitives/multiply-attribute.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/on-capture.test.ts b/packages/chess/src/modifiers/primitives/on-capture.test.ts index 06e9c27..93a18f6 100644 --- a/packages/chess/src/modifiers/primitives/on-capture.test.ts +++ b/packages/chess/src/modifiers/primitives/on-capture.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/on-captured.test.ts b/packages/chess/src/modifiers/primitives/on-captured.test.ts index f4ea306..0da9f3c 100644 --- a/packages/chess/src/modifiers/primitives/on-captured.test.ts +++ b/packages/chess/src/modifiers/primitives/on-captured.test.ts @@ -22,6 +22,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } 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 2087656..f868c50 100644 --- a/packages/chess/src/modifiers/primitives/on-check-delivered.test.ts +++ b/packages/chess/src/modifiers/primitives/on-check-delivered.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } 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 ba42f29..7f6773b 100644 --- a/packages/chess/src/modifiers/primitives/on-check-received.test.ts +++ b/packages/chess/src/modifiers/primitives/on-check-received.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/on-damaged.test.ts b/packages/chess/src/modifiers/primitives/on-damaged.test.ts index 9158446..1096c05 100644 --- a/packages/chess/src/modifiers/primitives/on-damaged.test.ts +++ b/packages/chess/src/modifiers/primitives/on-damaged.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/on-move.test.ts b/packages/chess/src/modifiers/primitives/on-move.test.ts index c2b88bf..c985ed5 100644 --- a/packages/chess/src/modifiers/primitives/on-move.test.ts +++ b/packages/chess/src/modifiers/primitives/on-move.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } 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 d0e3eb6..e879593 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 @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/on-promotion.test.ts b/packages/chess/src/modifiers/primitives/on-promotion.test.ts index addc485..d654011 100644 --- a/packages/chess/src/modifiers/primitives/on-promotion.test.ts +++ b/packages/chess/src/modifiers/primitives/on-promotion.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } 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 3b6791d..a01b708 100644 --- a/packages/chess/src/modifiers/primitives/on-turn-end.test.ts +++ b/packages/chess/src/modifiers/primitives/on-turn-end.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } 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 e42f9ed..3ccf7d4 100644 --- a/packages/chess/src/modifiers/primitives/on-turn-start.test.ts +++ b/packages/chess/src/modifiers/primitives/on-turn-start.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/override-promotion.test.ts b/packages/chess/src/modifiers/primitives/override-promotion.test.ts index 9cb5645..b3960a1 100644 --- a/packages/chess/src/modifiers/primitives/override-promotion.test.ts +++ b/packages/chess/src/modifiers/primitives/override-promotion.test.ts @@ -17,6 +17,7 @@ function makeContext(session: Session, pieceId: EntityId) { }, target: "self" as const, event: undefined, + bindings: new Map(), }; } diff --git a/packages/chess/src/modifiers/primitives/param-resolver.test.ts b/packages/chess/src/modifiers/primitives/param-resolver.test.ts new file mode 100644 index 0000000..9343a8a --- /dev/null +++ b/packages/chess/src/modifiers/primitives/param-resolver.test.ts @@ -0,0 +1,314 @@ +/** + * Tests for T12 — `resolveParams` walker. + * + * Coverage: + * - No-op on plain params (regression for pre-T12 primitives) + * - `{ $var }` resolution (success + unbound BindingError) + * - `{ "ctx-attr": { entity: "self", attr } }` resolution + * - `{ "ctx-attr": { entity: "chooser", ... } }` (set + unset) + * - `{ "ctx-attr": { entity: , ... } }` direct EntityId + * - `{ "ctx-build": { col, row } }` literal + via `$var` + * - `{ "ctx-build" }` out-of-range failures + * - Deep walk through nested objects + arrays + * - Recursive resolution: $var that points at an object containing + * another $var IS walked again (documented behaviour, not a bug) + */ +import { describe, expect, it } from "vitest"; +import { Session } from "@paratype/rete"; +import type { EntityId } from "@paratype/rete"; +import { ChessEngine } from "../../engine.js"; +import { + PRESET_STATE_ENTITY, +} from "../../schema.js"; +import type { BindingValue } from "./context.js"; +import { BindingError, resolveParams } from "./param-resolver.js"; +import type { PrimitiveApplyContext } from "./types.js"; + +function makeCtx(opts: { + bindings?: ReadonlyMap; + setupSession?: (s: Session) => void; + pieceId?: EntityId; +} = {}): { ctx: PrimitiveApplyContext; session: Session } { + const session = new Session(); + // Reserve id=1 as the default "self" piece so tests can populate + // facts on it before constructing the ctx if desired. + const pieceId = opts.pieceId ?? (session.nextId() as EntityId); + opts.setupSession?.(session); + const ctx: PrimitiveApplyContext = { + engine: new ChessEngine(), + session, + pieceId, + depth: 0, + descriptor: { id: "custom:test-param-resolver", type: "data", version: 1 }, + target: "self", + event: undefined, + bindings: opts.bindings ?? new Map(), + }; + return { ctx, session }; +} + +describe("resolveParams (T12) — no-op on plain params", () => { + it("returns primitive values unchanged", () => { + const { ctx } = makeCtx(); + expect(resolveParams("string", ctx)).toBe("string"); + expect(resolveParams(42, ctx)).toBe(42); + expect(resolveParams(true, ctx)).toBe(true); + expect(resolveParams(null, ctx)).toBe(null); + expect(resolveParams(undefined, ctx)).toBe(undefined); + }); + + it("returns plain params object structurally identical (regression for 22 existing primitives)", () => { + const { ctx } = makeCtx(); + expect(resolveParams({ attr: "Hp", delta: 5 }, ctx)).toEqual({ + attr: "Hp", + delta: 5, + }); + expect(resolveParams({ percentage: 35 }, ctx)).toEqual({ + percentage: 35, + }); + expect(resolveParams({ moveType: "capture" }, ctx)).toEqual({ + moveType: "capture", + }); + }); + + it("walks arrays without rewriting plain values", () => { + const { ctx } = makeCtx(); + expect(resolveParams([1, 2, "x"], ctx)).toEqual([1, 2, "x"]); + expect(resolveParams([{ a: 1 }, { b: "y" }], ctx)).toEqual([ + { a: 1 }, + { b: "y" }, + ]); + }); +}); + +describe("resolveParams — $var", () => { + it("resolves a numeric binding", () => { + const { ctx } = makeCtx({ bindings: new Map([["x", 99]]) }); + expect(resolveParams({ $var: "x" }, ctx)).toBe(99); + }); + + it("resolves a string binding", () => { + const { ctx } = makeCtx({ bindings: new Map([["color", "white"]]) }); + expect(resolveParams({ $var: "color" }, ctx)).toBe("white"); + }); + + it("throws BindingError when name is unbound, listing available bindings", () => { + const { ctx } = makeCtx({ + bindings: new Map([ + ["a", 1], + ["b", 2], + ]), + }); + let caught: unknown; + try { + resolveParams({ $var: "missing" }, ctx); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(BindingError); + expect((caught as Error).message).toContain("$missing"); + expect((caught as Error).message).toContain("$a"); + expect((caught as Error).message).toContain("$b"); + }); + + it("BindingError message lists '(none)' when no bindings exist", () => { + const { ctx } = makeCtx(); + expect(() => resolveParams({ $var: "x" }, ctx)).toThrow(/\(none\)/); + }); +}); + +describe("resolveParams — ctx-attr", () => { + it("resolves entity 'self' off ctx.pieceId", () => { + const { ctx } = makeCtx({ + setupSession: (s) => { + s.insert(1 as EntityId, "Color", "white"); + }, + }); + expect( + resolveParams({ "ctx-attr": { entity: "self", attr: "Color" } }, ctx), + ).toBe("white"); + }); + + it("resolves entity 'chooser' to king of LastModifierChooser color", () => { + const { ctx } = makeCtx({ + setupSession: (s) => { + s.insert(PRESET_STATE_ENTITY, "LastModifierChooser", "black"); + // Black king at id=2. + s.nextId(); // consume id=2 to mirror real session id allocation + s.insert(2 as EntityId, "PieceType", "king"); + s.insert(2 as EntityId, "Color", "black"); + }, + }); + // The walker resolves entity → 2 (the black king), then reads + // its Color attr → "black". + expect( + resolveParams( + { "ctx-attr": { entity: "chooser", attr: "Color" } }, + ctx, + ), + ).toBe("black"); + }); + + it("throws when entity 'chooser' is requested but LastModifierChooser is unset", () => { + const { ctx } = makeCtx(); + expect(() => + resolveParams( + { "ctx-attr": { entity: "chooser", attr: "Color" } }, + ctx, + ), + ).toThrow(/LastModifierChooser/); + }); + + it("throws when entity 'chooser' is set but no king of that color exists", () => { + const { ctx } = makeCtx({ + setupSession: (s) => { + s.insert(PRESET_STATE_ENTITY, "LastModifierChooser", "white"); + // Black king present, but chooser is white → no matching king. + s.nextId(); + s.insert(2 as EntityId, "PieceType", "king"); + s.insert(2 as EntityId, "Color", "black"); + }, + }); + expect(() => + resolveParams( + { "ctx-attr": { entity: "chooser", attr: "Color" } }, + ctx, + ), + ).toThrow(/no king of color 'white'/); + }); + + it("resolves entity given as numeric EntityId", () => { + const { ctx } = makeCtx({ + setupSession: (s) => { + s.nextId(); + s.insert(2 as EntityId, "Hp", 7); + }, + }); + expect( + resolveParams({ "ctx-attr": { entity: 2, attr: "Hp" } }, ctx), + ).toBe(7); + }); + + it("resolves entity via nested $var binding", () => { + const { ctx } = makeCtx({ + bindings: new Map([["target", 2]]), + setupSession: (s) => { + s.nextId(); + s.insert(2 as EntityId, "Hp", 12); + }, + }); + expect( + resolveParams( + { + "ctx-attr": { entity: { $var: "target" }, attr: "Hp" }, + }, + ctx, + ), + ).toBe(12); + }); + + it("throws when the resolved attr is unset", () => { + const { ctx } = makeCtx(); + expect(() => + resolveParams({ "ctx-attr": { entity: "self", attr: "Hp" } }, ctx), + ).toThrow(/is unset/); + }); +}); + +describe("resolveParams — ctx-build", () => { + it("computes Square = col + row*8 for literal inputs", () => { + const { ctx } = makeCtx(); + expect( + resolveParams({ "ctx-build": { col: 4, row: 3 } }, ctx), + ).toBe(28); // e4 + expect( + resolveParams({ "ctx-build": { col: 0, row: 0 } }, ctx), + ).toBe(0); + expect( + resolveParams({ "ctx-build": { col: 7, row: 7 } }, ctx), + ).toBe(63); + }); + + it("resolves col / row via nested $var", () => { + const { ctx } = makeCtx({ + bindings: new Map([ + ["c", 4], + ["r", 3], + ]), + }); + expect( + resolveParams( + { "ctx-build": { col: { $var: "c" }, row: { $var: "r" } } }, + ctx, + ), + ).toBe(28); + // Mixed literal + $var. + expect( + resolveParams( + { "ctx-build": { col: { $var: "c" }, row: 0 } }, + ctx, + ), + ).toBe(4); + }); + + it("throws when col or row is out of [0..7]", () => { + const { ctx } = makeCtx(); + expect(() => + resolveParams({ "ctx-build": { col: 8, row: 0 } }, ctx), + ).toThrow(/0\.\.7/); + expect(() => + resolveParams({ "ctx-build": { col: -1, row: 0 } }, ctx), + ).toThrow(/0\.\.7/); + expect(() => + resolveParams({ "ctx-build": { col: 0, row: 8 } }, ctx), + ).toThrow(/0\.\.7/); + }); + + it("throws when col or row is non-integer", () => { + const { ctx } = makeCtx(); + expect(() => + resolveParams({ "ctx-build": { col: 3.5, row: 0 } }, ctx), + ).toThrow(/0\.\.7/); + }); +}); + +describe("resolveParams — deep walk", () => { + it("substitutes resolver shapes nested at arbitrary depth", () => { + const { ctx } = makeCtx({ + bindings: new Map([["x", 5]]), + }); + const params = { + target: "self", + list: [{ $var: "x" }, 7, { nested: { $var: "x" } }], + square: { "ctx-build": { col: { $var: "x" }, row: 0 } }, + meta: { + deep: { deeper: { $var: "x" } }, + }, + }; + const out = resolveParams(params, ctx) as { + target: string; + list: unknown[]; + square: number; + meta: { deep: { deeper: number } }; + }; + expect(out.target).toBe("self"); + expect(out.list).toEqual([5, 7, { nested: 5 }]); + expect(out.square).toBe(5); + expect(out.meta.deep.deeper).toBe(5); + }); + + it("does NOT match shape when the magic key is one of multiple keys (must be exactly one)", () => { + // A user-authored params object that legitimately stores a key + // named "$var" alongside other fields is treated as a plain + // object — the magic shape requires EXACTLY one key. + const { ctx } = makeCtx({ + bindings: new Map([["x", 5]]), + }); + const out = resolveParams( + { $var: "x", extra: 1 } as unknown as Record, + ctx, + ) as Record; + // The "$var" string is preserved as data; "extra" stays. + expect(out).toEqual({ $var: "x", extra: 1 }); + }); +}); diff --git a/packages/chess/src/modifiers/primitives/param-resolver.ts b/packages/chess/src/modifiers/primitives/param-resolver.ts new file mode 100644 index 0000000..3a3c400 --- /dev/null +++ b/packages/chess/src/modifiers/primitives/param-resolver.ts @@ -0,0 +1,247 @@ +/** + * Param walker (T12) — runtime substitution of binding refs and + * context-attribute / context-build references inside primitive + * `params` trees, performed BEFORE the primitive's `apply()` is + * invoked. + * + * Three shape recognisers, each triggered ONLY when an object has + * exactly one matching key (so a plain `{ $var: ... }` field never + * collides with a primitive that legitimately stores a key starting + * with `$`): + * + * { $var: "name" } + * → `ctx.bindings.get("name")` + * → throws `BindingError` if the name is unbound + * + * { "ctx-attr": { entity, attr } } + * → `ctx.session.get(resolvedEntityId, attr)` + * → `entity` may be `"self"` | `"chooser"` | numeric EntityId | + * `{ $var: "name" }` (recursively resolved by `walk`) + * → throws if the resolved attr is `undefined` (unset) OR if + * `entity === "chooser"` but no `LastModifierChooser` is set + * + * { "ctx-build": { col, row } } + * → numeric Square = col + row * 8 + * → `col` / `row` may themselves be `{ $var }` shapes (walked first) + * → throws if either falls outside the integer range [0..7] + * + * Anything else (primitive value, plain object, array) is returned + * unchanged — arrays + plain-object values are deep-walked so a + * resolver shape buried at any depth is still substituted. + * + * ## Backward compatibility + * + * The 22 pre-T12 primitives never store any of the 3 magic keys in + * their params. Therefore the walker is a no-op for every existing + * primitive's `node.params` — the recursive deep-clone path simply + * reconstructs an equivalent object. Tests pin this byte-identical + * behaviour (regression: a plain Zod schema must still parse the + * resolved value identically). + * + * ## Why fail-loud + * + * Resolution failures (unbound `$var`, unset `ctx-attr`, out-of-range + * `ctx-build`) throw at apply time rather than producing + * `undefined` / sentinel values. The validator (T13) catches static + * shape errors; this module catches *runtime* hazards that depend on + * scope / session state and must surface to the caller. A primitive + * that swallowed `undefined` here would silently corrupt downstream + * facts — fail-loud keeps the runtime debuggable. + */ +import type { EntityId } from "@paratype/rete"; +import { PRESET_STATE_ENTITY, type ChessAttrKey } from "../../schema.js"; +import type { PrimitiveApplyContext } from "./types.js"; + +/** + * Thrown when a `$var` reference points at a name that isn't in + * `ctx.bindings`. The error message lists every name currently in + * scope so the author can see at a glance whether they mistyped or + * whether the binding hasn't been introduced yet at this point in + * the primitive tree. + */ +export class BindingError extends Error { + /** The unbound name (without the `$` prefix). */ + readonly name: string; + constructor(name: string, available: readonly string[]) { + const list = + available.length === 0 + ? "(none)" + : available.map((n) => `$${n}`).join(", "); + super( + `Binding '$${name}' is not in scope. Available bindings: ${list}.`, + ); + this.name = name; + // Subclass marker for `instanceof` checks across module boundaries. + Object.setPrototypeOf(this, BindingError.prototype); + } +} + +/** + * Recursively resolve binding / ctx references in a params tree. + * Returns a NEW value — the input is never mutated. Plain primitives + * (string/number/boolean/null) and unrecognised objects pass through + * with their structure preserved. + * + * @throws {BindingError} when a `$var` name is unbound. + * @throws {Error} when a `ctx-attr` entity is unresolvable, the attr + * is unset on the resolved entity, or a `ctx-build` col/row falls + * outside the integer range [0..7]. + */ +export function resolveParams( + params: unknown, + ctx: PrimitiveApplyContext, +): unknown { + return walk(params, ctx); +} + +function walk(node: unknown, ctx: PrimitiveApplyContext): unknown { + if (node === null || typeof node !== "object") return node; + if (Array.isArray(node)) return node.map((item) => walk(item, ctx)); + + const obj = node as Record; + const keys = Object.keys(obj); + + // Single-key magic-shape recognition. We require EXACTLY one key so + // a plain object that happens to contain `$var` alongside other + // fields isn't accidentally treated as a binding ref. + if (keys.length === 1) { + const only = keys[0]!; + + if (only === "$var") { + const name = obj.$var; + if (typeof name !== "string") { + throw new Error( + `$var: name must be a string, got ${typeof name} (${JSON.stringify(name)})`, + ); + } + if (!ctx.bindings.has(name)) { + throw new BindingError(name, Array.from(ctx.bindings.keys())); + } + return ctx.bindings.get(name); + } + + if (only === "ctx-attr") { + const inner = obj["ctx-attr"]; + if (inner === null || typeof inner !== "object") { + throw new Error( + `ctx-attr: payload must be an object, got ${typeof inner}`, + ); + } + const { entity, attr } = inner as { + entity?: unknown; + attr?: unknown; + }; + if (typeof attr !== "string") { + throw new Error( + `ctx-attr.attr: must be a string, got ${typeof attr}`, + ); + } + const entityId = resolveEntity(entity, ctx); + const value = ctx.session.get( + entityId, + attr as ChessAttrKey, + ); + if (value === undefined) { + throw new Error( + `ctx-attr: entity ${entityId as number}.${attr} is unset`, + ); + } + return value; + } + + if (only === "ctx-build") { + const inner = obj["ctx-build"]; + if (inner === null || typeof inner !== "object") { + throw new Error( + `ctx-build: payload must be an object, got ${typeof inner}`, + ); + } + const { col: rawCol, row: rawRow } = inner as { + col?: unknown; + row?: unknown; + }; + const col = walk(rawCol, ctx); + const row = walk(rawRow, ctx); + if (typeof col !== "number" || typeof row !== "number") { + throw new Error( + `ctx-build: col/row must resolve to numbers, got col=${typeof col} row=${typeof row}`, + ); + } + if ( + !Number.isInteger(col) || + !Number.isInteger(row) || + col < 0 || + col > 7 || + row < 0 || + row > 7 + ) { + throw new Error( + `ctx-build: col/row out of [0..7] (col=${col}, row=${row})`, + ); + } + return col + row * 8; + } + } + + // Plain object — recurse on each value. + const out: Record = {}; + for (const [k, v] of Object.entries(obj)) { + out[k] = walk(v, ctx); + } + return out; +} + +/** + * Resolve a `ctx-attr` entity selector to a concrete EntityId. + * + * "self" — `ctx.pieceId`. + * "chooser" — looks up `LastModifierChooser` on `PRESET_STATE_ENTITY` + * (T14 stub) and returns the king of that color. T14 + * writes the chooser color at the start of every + * `applyCustomDescriptor` call. Future apply-modifier + * action handler may refine this to "any piece of + * chooser color". + * number — passed through verbatim (allows authors to target a + * specific piece id, e.g. PRESET_STATE_ENTITY (-1) for + * preset-state attrs). + * { $var } — recursively resolved via `walk` (must yield a number). + */ +function resolveEntity( + entity: unknown, + ctx: PrimitiveApplyContext, +): EntityId { + if (entity === "self") return ctx.pieceId; + if (entity === "chooser") { + const color = ctx.session.get( + PRESET_STATE_ENTITY, + "LastModifierChooser", + ); + if (color === undefined) { + throw new Error( + "ctx-attr entity 'chooser': no LastModifierChooser set on PRESET_STATE_ENTITY (chooser tracking not initialised)", + ); + } + // Find the king of the chooser color. Walks Color facts and + // cross-references PieceType — same id-of-color resolution + // pattern used by `resolveByRelation` in context.ts. + for (const f of ctx.session.allFacts()) { + if (f.attr !== "PieceType" || f.value !== "king") continue; + if ((f.id as number) <= 0) continue; + const idColor = ctx.session.get(f.id, "Color"); + if (idColor === color) return f.id; + } + throw new Error( + `ctx-attr entity 'chooser' (${color}): no king of color '${color}' found in session`, + ); + } + if (typeof entity === "number") return entity as EntityId; + // Could be `{ $var: "..." }` — let `walk` resolve it; result must + // be numeric to be a valid EntityId. + const resolved = walk(entity, ctx); + if (typeof resolved !== "number") { + throw new Error( + `ctx-attr.entity: must resolve to a numeric EntityId or "self"/"chooser", got ${typeof resolved} (${JSON.stringify(resolved)})`, + ); + } + return resolved as EntityId; +} diff --git a/packages/chess/src/modifiers/primitives/reflect-damage.test.ts b/packages/chess/src/modifiers/primitives/reflect-damage.test.ts index 4ccba67..60a1ef4 100644 --- a/packages/chess/src/modifiers/primitives/reflect-damage.test.ts +++ b/packages/chess/src/modifiers/primitives/reflect-damage.test.ts @@ -17,6 +17,7 @@ function makeContext(session: Session, pieceId: EntityId) { }, target: "self" as const, event: undefined, + bindings: new Map(), }; } diff --git a/packages/chess/src/modifiers/primitives/seed-attribute.test.ts b/packages/chess/src/modifiers/primitives/seed-attribute.test.ts index e83f629..f7634b7 100644 --- a/packages/chess/src/modifiers/primitives/seed-attribute.test.ts +++ b/packages/chess/src/modifiers/primitives/seed-attribute.test.ts @@ -21,6 +21,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/set-capture-flag.test.ts b/packages/chess/src/modifiers/primitives/set-capture-flag.test.ts index d0f5f41..4a4ab34 100644 --- a/packages/chess/src/modifiers/primitives/set-capture-flag.test.ts +++ b/packages/chess/src/modifiers/primitives/set-capture-flag.test.ts @@ -22,6 +22,7 @@ function makeContext(): { ctx: PrimitiveApplyContext; session: Session } { }, target: "self", event: undefined, + bindings: new Map(), }; return { ctx, session }; } diff --git a/packages/chess/src/modifiers/primitives/types.ts b/packages/chess/src/modifiers/primitives/types.ts index e7e60bd..72718b1 100644 --- a/packages/chess/src/modifiers/primitives/types.ts +++ b/packages/chess/src/modifiers/primitives/types.ts @@ -2,7 +2,11 @@ import type { EntityId, Session } from "@paratype/rete"; import type { ZodType } from "zod"; import type { ChessEngine } from "../../engine.js"; import type { ChessAttrKey } from "../../schema.js"; -import type { PrimitiveEvent, TargetResolver } from "./context.js"; +import type { + BindingValue, + PrimitiveEvent, + TargetResolver, +} from "./context.js"; /** * T3 primitive ids (ADR-2). @@ -84,6 +88,19 @@ export interface PrimitiveApplyContext { * applies and for triggers that don't carry per-event payload. */ readonly event: PrimitiveEvent | undefined; + /** + * Lexically-scoped bindings (T11). Iteration primitives (T31-T35) + * and request-choice (T47) introduce names into this map via the + * `withBinding(ctx, name, value)` helper from `./context.js`, which + * returns a NEW context with the addition; the original `bindings` + * map is never mutated. Inner primitives read via + * `ctx.bindings.get(name)`. + * + * Default at every construction site: `new Map()`. The 22 + * pre-T11 primitives don't consult bindings, so the empty default + * preserves their behaviour byte-identically. + */ + readonly bindings: ReadonlyMap; } /** diff --git a/packages/chess/src/modifiers/triggers.ts b/packages/chess/src/modifiers/triggers.ts index f1767b5..3d031b2 100644 --- a/packages/chess/src/modifiers/triggers.ts +++ b/packages/chess/src/modifiers/triggers.ts @@ -66,8 +66,10 @@ import { PIECE_TYPE_REGISTRY } from "../presets/piece-type-registry.js"; import { PRIMITIVE_REGISTRY } from "./primitives/registry.js"; import { resolveTargets, + type BindingValue, type PrimitiveEvent, } from "./primitives/context.js"; +import { resolveParams } from "./primitives/param-resolver.js"; import type { EffectPrimitiveNode, PrimitiveApplyContext, @@ -119,6 +121,7 @@ function runPrimitives( nodes: readonly EffectPrimitiveNode[], depth: number, event?: PrimitiveEvent, + bindings: ReadonlyMap = new Map(), ): void { if (depth > 8) return; // hard runtime cap, mirrors validator for (const node of nodes) { @@ -141,18 +144,34 @@ function runPrimitives( // concern, not a runner concern. target: "self", event, + // T11: bindings flow inward through recursion. Trigger + // dispatchers seed an empty map at hook entry; iteration / + // request-choice primitives extend it via `withBinding` before + // re-entering `runPrimitives` for nested children. + bindings, }; - primitive.apply(ctx, node.params); + // T12: resolve `$var` / `ctx-attr` / `ctx-build` shapes inside + // params BEFORE handing them to the primitive's apply(). Existing + // primitives never store these magic keys, so the walker is a + // no-op for their params (returns a structurally-identical clone). + const resolvedParams = resolveParams(node.params, ctx); + primitive.apply(ctx, resolvedParams); if (primitive.childPrimitives === undefined) continue; let children: readonly EffectPrimitiveNode[] = []; try { + // childPrimitives() introspects the ORIGINAL params — the + // structural shape (which inner primitive lists exist) is + // independent of the runtime-resolved values. Resolution + // happens per-recursion when the children themselves apply. children = primitive.childPrimitives(node.params); } catch { children = []; } if (children.length > 0) { - runPrimitives(engine, pieceId, children, depth + 1, event); + // Thread the SAME bindings through nested children so a name + // introduced by an outer iteration primitive remains in scope. + runPrimitives(engine, pieceId, children, depth + 1, event, bindings); } } } @@ -501,6 +520,10 @@ export function fireOnCapturedHooks( descriptor: { id: "__trigger__", type: "data", version: 1 }, target: hook.target, event, + // T11: empty bindings — on-captured hook entry has no enclosing + // iteration scope. Inner primitives that introduce bindings + // extend via `withBinding` once `runPrimitives` recurses. + bindings: new Map(), }; const targets = resolveTargets(resolverCtx, hook.target); for (const targetId of targets) { diff --git a/packages/chess/src/schema.test.ts b/packages/chess/src/schema.test.ts index 2345641..ce964cd 100644 --- a/packages/chess/src/schema.test.ts +++ b/packages/chess/src/schema.test.ts @@ -197,3 +197,20 @@ describe("T8 movement-replacement attrs (Wave 2)", () => { expect(typeof two.value).toBe("number"); }); }); + +// ─── T14: chooser tracking attr ──────────────────────────────────── +// +// `LastModifierChooser` records the color of the player who triggered +// a descriptor activation. Stored on `PRESET_STATE_ENTITY` (id -1) so +// the param walker (T12) can resolve `ctx-attr: { entity: "chooser", +// attr: "Color" }` to a concrete PieceColor. Pure schema/registration +// round-trip; the actual write happens in `applyCustomDescriptor`. +describe("T14 chooser tracking attr", () => { + it("LastModifierChooser stores a PieceColor value", () => { + const w = chessFact(mkId(-1), "LastModifierChooser", "white"); + const b = chessFact(mkId(-1), "LastModifierChooser", "black"); + expect(w.value).toBe("white"); + expect(b.value).toBe("black"); + expect(w.attr).toBe("LastModifierChooser"); + }); +}); diff --git a/packages/chess/src/schema.ts b/packages/chess/src/schema.ts index ce89392..69a82bd 100644 --- a/packages/chess/src/schema.ts +++ b/packages/chess/src/schema.ts @@ -229,6 +229,19 @@ export interface ChessAttrMap { * Chebyshev radius by Wave 7 movement-gen. */ KingExtraReach: number; + /** + * T14 — chooser-tracking stub. Records the color of the player who + * most recently triggered an `apply-modifier` action (i.e. the + * "chooser" of the descriptor). Stored on `PRESET_STATE_ENTITY` so + * the param walker (T12) can resolve `ctx-attr: { entity: "chooser", + * attr: "Color" }` to a PieceColor at runtime. + * + * Schema slot reserved here; the actual write happens when the + * `apply-modifier` PlayerAction handler lands (future task). Until + * then the fact may be absent — `ctx-attr` callers must treat + * "undefined" as "no chooser available" and surface a runtime error. + */ + LastModifierChooser: PieceColor; } export type ChessAttrKey = keyof ChessAttrMap;