houserules/.sisyphus/notepads/thressgame-coverage/learnings.md
Joey Yakimowich-Payne e290f350ad
feat(thressgame-coverage): Wave 5 (7 imperative primitives)
- T21: place-piece — calls engine.spawnPiece on resolved square
- T22: destroy-piece — retracts piece facts; enqueues on-captured
- T23: move-piece — updates Position + HasMoved; enqueues on-move + on-moved-onto-square
- T24: swap-pieces — atomic Position swap; enqueues 2 on-move events
- T25: convert-piece-type — changes PieceType; enqueues on-promotion (with previous-equality short-circuit)
- T26: set-piece-attr — generic attr insert (parity descriptors use heavily); lifetime field accepted but ignored in V1
- T27: cancel-capture — sets CaptureCancelled flag on GAME_ENTITY; rejects outside on-captured context

T20 test fix: synthetic suppressTriggers test moved from 'swap-pieces' kind (T24 took it) to 'spawn-marker-pair' (Wave 6 / T29 territory).

Registry: 26 -> 33 primitives. Tests: 2120 -> 2225 (+105). bun run check exit 0.
2026-04-26 10:25:58 -06:00

104 KiB
Raw Permalink Blame History

Thressgame-Coverage Executor Learnings

[2026-04-26T00:27:55Z] T1 baseline fixture

Captured Baseline

  • Test count: 1324 tests across 113 test files
  • Primitive files enumerated: 28 distinct .test.ts files in packages/chess/src/modifiers/primitives/
  • Total test+expect assertions: 21,846 expect() calls recorded across unit test suite
  • Regression baseline: ≥ 1324 tests required to pass regression suite

Key Numbers

Metric Value
files 113
tests 1324
primitive test files 28
min test count 2 (registry-count.test.ts)
max test count 14 (on-moved-onto-square.test.ts)
total primitive tests 152
total primitive expects 267

Fixture Files Created

  1. ✅ baseline-test-count.json — snapshot of 1324 tests, 113 files, ISO8601 timestamp
  2. ✅ baseline-primitive-tests.json — array of 28 primitives with kind, file, testCount, expectCount
  3. ✅ baseline-regression.test.ts — 4-test Vitest regression guard
    • Validates JSON shapes
    • Asserts baseline tests ≥ 1324
    • File-by-file regression check for each primitive

Verification Status

  • ✅ bun test baseline-regression.test.ts → 4 pass, 0 fail (exit 0)
  • ✅ bun run check → 1961 tests passed across 167 files (post-creation check)
  • ✅ No breakage introduced — all fixtures are non-intrusive read-only snapshots

Dependencies & Handoff

  • Blocks: Waves 5–10 implementation tasks (primitive mutations will now trigger regression detection)
  • Blocked By: T0 ✅ (complete)
  • Evidence: .sisyphus/evidence/task-1-baseline-test.txt contains full bun test output

[2026-04-26T06:29:52Z] T1 baseline correction

Atlas verification fixes:

  1. ✅ Corrected baseline numbers — used bun run check (canonical workspace command, not bun test from root)
    • Files: 113 → 167 (includes all packages: rete, chess, server)
    • Tests: 1324 → 1961 (full workspace count)
  2. ✅ Real timestamp — 2026-04-26T00:00:00Z → 2026-04-26T06:29:52Z

Critical lesson: The canonical test command in this repo is bun run check, which validates the entire workspace. Direct bun test from root or subpackages gives incomplete counts. Future regression baselines must use bun run check output only.

[2026-04-26T07:15:00Z] T3 state-hash util

Implementation Summary

  • File: packages/chess/src/util/state-hash.ts — 48 lines, exports hashEngineState(engine: ChessEngine): string
  • Algorithm: Deterministic SHA256 hash of session state via session.allFacts()
  • Ordering: Facts are already sorted by allFacts() as [id asc, attr asc]
  • Session API used: session.allFacts() returns array of { id, attr, value } tuples

Key Insight for T2 (Determinism Harness)

The Session API for iterating facts is simple:

const facts = engine.session.allFacts();
for (const fact of facts) {
  const id = fact.id as number;
  const attr = fact.attr;
  const value = fact.value;
}

No custom iteration method needed — allFacts() is public and sorted deterministically. T2 can reuse this exact pattern when building the determinism test harness.

Tests Added (4)

  1. ✅ 64-char hex validation
  2. ✅ Identical engines → identical hash
  3. ✅ One fact difference → different hash
  4. ✅ Insertion-order independence (same entity, different attr insertion order)

Verification

  • ✅ bun test packages/chess/src/util/state-hash.test.ts → 4 pass, 0 fail (exit 0)
  • ✅ bun run check → 1965 tests passed (1961 → 1965, +4 tests)
  • ✅ Evidence: .sisyphus/evidence/task-3-state-hash.txt

T5 Verdict: Binding Shape — CLEAN

Audited 2026-04-26. Zero -prefixed keys or values found across:

  • 23 primitive implementations
  • 66+ paramsSchema definitions
  • All descriptor JSON fixtures

Binding shape { $var: "..." } for T12 is safe to implement.

[2026-04-26T06:36:00Z] T2 determinism harness

Implementation Summary

  • Files: packages/chess/src/__fixtures__/determinism/{harness.ts,harness.test.ts} (97 + 84 lines)
  • Public API: runDeterminismCheck(setup: SetupFn, moves: readonly MoveFn[], iterations = 100): DeterminismResult
    • SetupFn = () => ChessEngine — fresh-engine factory invoked once per iteration
    • MoveFn = (engine: ChessEngine) => void — applies one mutation; closes over its own move lookup
    • DeterminismResult = { hash, matches, iterations, mismatchAt?, mismatchHash? }
  • Algorithm: For each iteration build a fresh engine via setup(), fold moves over it, hash via hashEngineState(), compare to iter-0 reference; bail early on first divergence and surface mismatchAt + mismatchHash.

Move API used in sanity test (REUSE THIS in Wave 8/10)

const legal = engine.getAllLegalMoves();
const m = legal.find((mv) => mv.from === 12 && mv.to === 28);
engine.applyMove(m!);

Square indices are 0-indexed: file + rank*8 (e2 = 12, e4 = 28, e7 = 52, e5 = 36). This pattern is identical to what triggers.test.ts:73-82 and piece-hp.test.ts:109+ already use — keep using it.

Measured Timing (100-iteration sanity test)

Scenario Wall-clock
5 tests in harness.test.ts (incl. 100-iter sanity + 100-iter default + 10 + 5 negative) 1.70s
100-iter sanity test in isolation (Vitest reports test+setup) 1.67s
Plan budget < 5000ms
Headroom ~3.3s (3× margin)

The 100-iteration loop itself (excluding Vitest startup) executes well under the 5000ms cap. new ChessEngine() boot dominates — each iteration constructs a full preset stack from scratch, but it's still cheap enough.

Tests Added (5)

  1. ✅ Standard chess deterministic across 100 iterations (e2-e4, e7-e5)
  2. ✅ Default iterations = 100 when not specified
  3. ✅ Empty moves list still hashes the fresh engine deterministically (10 iters)
  4. ✅ Rejects iterations < 1 (0 and -1)
  5. ✅ Negative test: detects synthetic non-determinism (extra spawnPiece on iter 1+) → asserts matches=false, mismatchAt=1

Verification

  • ✅ bun test packages/chess/src/__fixtures__/determinism/harness.test.ts → 5 pass, 0 fail (exit 0)
  • ✅ bun run check → 1970 tests passed (1965 → 1970, +5 tests)
  • ✅ LSP diagnostics: clean on both files
  • ✅ Evidence: .sisyphus/evidence/task-2-determinism-harness.txt

Reusability Contract for Downstream Waves

  • Wave 8 (RNG primitives): pass a setup that includes a RngSeed insertion (or rely on game-start seed init from T9), and moves that exercise with-probability / random-pick. The harness will catch any RNG leak across iterations (e.g. shared module-level state).
  • Wave 10 (parity tests): each parity descriptor's e2e fixture can be wrapped — setup builds engine with the descriptor active, moves replays the canonical move sequence. Determinism = N=100 byte-identical state hashes.
  • No internal mutation of inputs: the harness never mutates setup, moves, or any element. Caller closures (e.g. RNG-using descriptors) are responsible for being deterministic; harness only measures.

[2026-04-26T00:41:04-06:00] T4 position audit summary

Counts

  • Total grep hits ("Position" string-literal): 241 lines across 94 files
  • Typed-identifier Position references (excluding strings): 115 (mostly comments/docstrings/type-defs/test-IT-titles — only schema.ts:65 is the type alias, context.ts:151 is one cast)
  • Production callsites enumerated: 75 numbered rows in position-audit.md
  • Test-file callsites: ≈137 (catalogued in §Tests of audit, NOT individually risk-graded)

Classification (production)

  • (a) PIECE-ASSUMING READ: 8 — 0 unsafe (4 already piece-typed via PieceType-anchored callers, 3 are API-LAYER contracts to document, 1 chained from upstream)
  • (b) WRITE: 15 — all inherently safe; markers writing their own Position is correct
  • (c) ITERATION: 22 — 9 unsafe + 5 partial-safe (markers without Color naturally skipped but explicit EntityKind filter recommended for clarity)
  • Static / typed / docstring / false-positive: 5

Downstream-task fix list (the audit's deliverable)

T6 owner (schema EntityKind seeding) MUST add EntityKind === "piece" filter to:

  • engine.ts:1680 (getPieceAt private)
  • rules/board-queries.ts:38, 46, 56, 78 (isPieceAt, isEnemyAt, isAllyAt, getPieceAt public)
  • rules/capture.ts:94 (canCapture)
  • rules/check.ts:173 (clearSquare in self-check what-if)
  • rules/stalemate.ts:77, 87 (what-if construction)
  • rules/draws.ts:40 (computePositionHash — markers MUST NOT contribute to repetition hash)
  • modifiers/apply.ts:234, 502 (snapshotPositions, buildSquareIndex)
  • modifiers/primitives/context.ts:148 (resolveBySquares)
  • presets/queen-splits.ts:72, explosive-rook.ts:34, knight-immunity.ts:16, poisoned-squares.ts:45, weak-dual-king.ts:199

T7 owner (aura compute) MUST address:

  • modifiers/auras.ts:99-107 (collectPiecesWithPositions) — DESIGN CALL per T0 line 55: markers must participate as aura sources/targets. Likely rename to collectPositionedEntities and split source/target classification by EntityKind. Decide whether marker entities accept AuraContributions writes.

Wave-3+ (UI rendering):

  • ui/Board.tsx:103 — add parallel marker-rendering pass for EntityKind === 'marker' entities
  • ui/GameView.tsx — surface marker overlays (separate visual tier)

API-LAYER docstring updates only (no behaviour change):

  • rules/board-queries.ts:65 (getPiecePosition)
  • modifiers/source.ts:26 (getModifierSource)
  • All preset getExtraMoves consumers (knights-leap-twice, bishops-ignore-color, rook-warp, wrap-board, bouncing-pieces, bouncing-pieces-2)

Open Questions Logged for T6/T7 leads

  1. Marker-as-capture-target: confirm T22 contract that dealDamage(markerId, …) is a no-op so queen-splits.ts:106 and explosive-rook.ts:57 chained safety holds.
  2. Aura-target eligibility: do markers receive AuraContributions? T7 must decide.
  3. Snapshot what-if helpers (check.ts:snapshotSession, stalemate.ts inline copy, weak-dual-king.ts:snapshotPieces): once EntityKind lands in schema, isPieceAttr will (correctly) return true for it, so markers' EntityKind facts flow into temp sessions. Per-helper clearSquare analogues then need EntityKind filter — already itemized above.

Tests

~137 test-file hits identified by file. Strategy: when T6 lands, engine.spawnPiece should auto-seed EntityKind: "piece" so existing test setup paths inherit the discriminator without per-test edits. Test-helper placePiece/makePiece shapes (e.g. rules/pawn.test.ts:20, rules/sliding.test.ts:24, presets/test-utils.ts:94, etc.) will need EntityKind: "piece" seeded if they bypass spawnPiece.

Verification

  • grep -rn '"Position"' packages/chess/src/ | wc -l → 241 (matches evidence)
  • Audit file: 342 lines, 75 numbered callsite rows
  • Evidence: .sisyphus/evidence/task-4-position-audit.txt

[2026-04-26T08:21Z] T6 entity / marker / RNG attrs

What landed

  • 3 named type aliases ABOVE ChessAttrMap in packages/chess/src/schema.ts: EntityKindValue, MarkerKindValue, MarkerLifetimeValue. All exported so downstream tasks (T7 EntityKind filters; T10 marker factory; T19 lifetime decrementer; RNG primitives) can import them.
  • 7 attrs appended to ChessAttrMap in declaration order: EntityKind, MarkerKind, MarkerLifetime, MarkerOwner, MarkerLinks, RngSeed, RngStream.
  • 7 registerAttrConsumer(...) calls appended to the END of the block in packages/chess/src/modifiers/apply.ts (lines 116-122). Placement at end-of-block minimises merge friction with T8 (which the brief warned will also append here).
  • 7 new tests in packages/chess/src/schema.test.ts under describe T6 marker + RNG attrs.

Import paths verified (use these in T7/T8/T10)

  • EntityId — import type { EntityId } from "@paratype/rete" (already imported at schema.ts:9; no re-import needed by future tasks that pull from ./schema.js).
  • New value types — import { type EntityKindValue, type MarkerKindValue, type MarkerLifetimeValue } from "./schema.js".

Conventions followed

  • MarkerOwner value type is PieceColor ("white" | "black"); absence of the fact encodes "no owner" — do NOT insert undefined. (Brief rationale was correct.)
  • MarkerLifetime field names are kind + expiresAtMove exactly, matching decisions.md:44. expiresAtMove is an ABSOLUTE move count comparison (T19 will compare against FullmoveNumber or equivalent).
  • MarkerKind is the locked 8-value union — no string widening.

Surprises / gotchas

  • Brief said apply.ts block is at packages/chess/src/modifiers/custom/apply.ts lines 95-113; ACTUAL location is packages/chess/src/modifiers/apply.ts lines 95-114. The custom/apply.ts file is the descriptor walker, not the consumer registration site. Mirror this when wiring T7/T8.
  • schema.test.ts uses chessFact() factory + EntityId brand cast via mkId(n) helper — NOT a live Session. Mirror this lighter pattern; full Session round-trip belongs in engine-level integration tests, not the pure-schema test file.
  • bun run check runs typecheck → lint → vitest (workspace mode). Vitest project name for chess is |chess|. Test count after T6 = 1990 (was 1970 baseline; +20 because parallel tasks have already landed in the same wave window — the +7 from T6 alone is included).
  • assertSeedConsumerIntegrity only asserts that EVERY primitive-seeded attr has a consumer; it does NOT complain about consumers without seed-side coverage. So registering RngSeed/RngStream consumers before any RNG primitive seeds them is safe and forward-compatible.

Verification

  • bun test packages/chess/src/schema.test.ts → 18 pass / 0 fail
  • bunx vitest run schema.test.ts manifest.test.ts → 23 pass
  • bun run check → exit 0, 1990 tests across 170 files
  • Evidence: .sisyphus/evidence/task-6-attrs.txt

[2026-04-26T08:24Z] T9 RNG (Mulberry32 + engine.rng + GAME_ENTITY seeding)

What landed

  • packages/chess/src/util/rng.ts — class SeededRng (Mulberry32) + deriveSeedFromGameId(gameId) (FNV-1a 32-bit). No deps; verbatim reference Mulberry32 constants — do NOT "optimize" the bit pattern, it's load-bearing for cross-impl determinism.
  • packages/chess/src/util/rng.test.ts — 13 tests / 5086 expects. Includes 1000-iter range guards on next() and nextInt(N), golden-sequence regression for seed=1 (any value change is a plan- amending event), pick/empty throw, FNV uint32 invariant.
  • packages/chess/src/engine.ts —
    1. New gameId?: string field on EngineOptions.
    2. RngSeed/RngStream seeded UNCONDITIONALLY in the constructor, BEFORE applyProfileToSession / preset activation, so any onActivate hook that draws (future RNG-shuffled starting positions, etc.) sees initialized facts. Default seed when no gameId = deriveSeedFromGameId(undefined) = 1.
    3. engine.rng() returns a handle with next/nextInt/pick. EACH call advances the persistent RngStream by 1 by re-reading the fact, constructing a fresh SeededRng(seed + currentStream), then insert(GAME_ENTITY, "RngStream", currentStream + 1).
    4. engine.setRngSeed(numberOrString) resets stream to 0 and overrides seed; for tests / late-binding server boot.

RNG init location decision

  • Constructor, NOT integration preset boot. Rationale: RngSeed/RngStream are GAME-level facts (not modifier-related); the constructor is the only call site guaranteed to fire for every engine regardless of whether a profile / preset is active. The __modifier-profile-integration__ preset only activates when a profile is supplied — putting RNG init there would make any profileless engine throw at first engine.rng() call.
  • This decision keeps T9 independent of T6's preset boot path; no edits to modifiers/custom/apply.ts or modifiers/apply.ts needed.

engine.rng() return shape (for Wave 7 — T36 with-probability, T37 random-pick)

{
  next(): number;       // [0, 1) — advances stream by 1
  nextInt(max: number): number;  // [0, max) — advances stream by 1
  pick<T>(arr: readonly T[]): T; // throws on empty — advances stream by 1
}

Critical for T36/T37: ONE call = ONE stream advance = ONE draw. Don't cache the handle then call next() 5 times expecting the same value; each call re-reads the stream. To draw N times for one effect, call N times — the stream offset is what makes each distinct.

Subtleties for downstream RNG consumers

  • The "draws-before-suspension" invariant (decisions.md:140) is enforced in the VALIDATOR (T19), not in the engine — the RNG itself doesn't know about suspension. T36's with-probability must call engine.rng().next() BEFORE selecting then/else arm, and the validator must reject any request-choice nested inside with-probability.
  • setRngSeed mid-game IS allowed but breaks replay-from-this- point. Server boot path: pass gameId to constructor for the authoritative session; clients receive RngSeed/RngStream facts via the standard fact-sync (no special-casing needed — they're just facts on GAME_ENTITY).
  • assertSeedConsumerIntegrity (T6) already covers RngSeed/RngStream consumer registration; T9 added zero registerAttrConsumer calls (per brief — would have raced T6).

Verification

  • bun test packages/chess/src/util/rng.test.ts → 13 pass / 0 fail
  • bun test packages/chess/src/engine-presets.test.ts → 24 pass / 0 fail (added 9 rng integration tests)
  • bun run check → exit 0, 1999 tests across 170 files (added 13 unit + 9 engine-integration; net delta vs previous T6 baseline reflects parallel tasks landing in the same wave)
  • Evidence: .sisyphus/evidence/task-9-rng.txt

[2026-04-26T08:30:00Z] T8 movement-replacement attrs

Added (Wave 2 schema/registration only)

5 new attrs on ChessAttrMap in packages/chess/src/schema.ts:

Attr Type Scope
MovesAs PieceType per-piece (override native pattern)
MovesAlsoAs PieceType per-piece (additive secondary pattern)
SlideMustBeMaxDistance boolean per-piece OR GAME_ENTITY
BlockAllExceptKing boolean GAME_ENTITY only
KingExtraReach number per-piece

5 matching registerAttrConsumer(...) calls appended to packages/chess/src/modifiers/apply.ts (location confirmed consistent with T6 — NOT modifiers/custom/apply.ts). Append-only edit; T6's 7 RNG/marker registrations were already present, so no race / overwrite occurred.

Confirmation re: apply.ts location

T6's earlier learnings entry was correct: the file is packages/chess/src/modifiers/apply.ts (line ~88+ holds the existing registerAttrConsumer block). The modifiers/custom/apply.ts file exists but is for custom-modifier application, not consumer registration. T8 followed T6's pattern.

Scope guardrails honoured

  • NO move-gen wiring touched. MovesAs / MovesAlsoAs / etc. are pure schema entries; the readers land in Wave 7 (T40 set-moves-as, T41 pawn-pushes-pieces) and Wave 10 (T64 ice-physics test consumes SlideMustBeMaxDistance).
  • NO defaults seeded. Move-gen treats absence-of-fact as "use natural movement", per the brief.
  • NO validator gate on MovesAs: "king" — V1 just stores; the reject-on-king restriction is T40's job (validator wave).

Test deltas

  • Added 5 tests in packages/chess/src/schema.test.ts under describe("T8 movement-replacement attrs (Wave 2)") mirroring T6's chessFact round-trip pattern.
  • Schema test file: 18 → 23 pass.
  • Workspace total: 1999 → 2006 pass (delta +7 = my 5 + 2 from expanding existing T6-style "every PieceType" loops).

Verification

  • bun test packages/chess/src/schema.test.ts → 23 pass / 0 fail.
  • bun test packages/chess/src/modifiers/primitives/consumer-integration.test.ts → 6 pass / 0 fail (no regression — assertSeedConsumerIntegrity happy with the 5 new declarations).
  • bun run check → exit 0, 2006 tests / 170 files.
  • Evidence: .sisyphus/evidence/task-8-movement-attrs.txt

[2026-04-26T14:29Z] T7 aura compute + markers

What landed

  • packages/chess/src/modifiers/auras.ts — added exported getEntityKind(session, id) helper + renamed internal walker collectPiecesWithPositions → collectPositionedEntities. Walker now filters via the EntityKind discriminator (admits "piece" and "marker") instead of the raw id > 0 rejection. Net behavior change for current callers = none (no markers exist yet); structural surface for T10 / T18 / T28 = ready.
  • packages/chess/src/modifiers/auras.test.ts — +2 tests under new describe aura compute — markers participate (T7). All 10 prior tests pass byte-identical.

CROSS-CUTTING DECISION — EntityKind default policy

  • Locked policy: EntityKind defaults to "piece" when absent AND PieceType is set. Returns undefined if neither is set.
  • Rationale: engine.spawnPiece (engine.ts:841-869) does NOT seed EntityKind (T6 only added the schema slot). A retroactive sweep through spawnPiece + every test fixture that bypasses the factory would be a Wave-2-scope-creep. Defaulting to "piece" keeps every legacy piece visible to EntityKind consumers, while markers (T10) MUST insert EntityKind = "marker" explicitly.
  • Every later EntityKind consumer MUST honor this policy — call getEntityKind directly. Specifically:
    • T10 (marker factory): set EntityKind = "marker" explicitly. Don't rely on absence.
    • T18 (on-piece-entered-marker): use getEntityKind to confirm the entering entity is a piece, and probe markers at the destination via EntityKind === "marker".
    • T28 (spawn-marker primitive): same — explicit "marker" fact.
    • Audit fixes from position-audit.md (T6/T7 column): when adding EntityKind === "piece" filters, prefer getEntityKind(session, id) === "piece" to inherit the legacy default.
  • Once a future cleanup sweep adds EntityKind = "piece" to engine.spawnPiece, the PieceType-fallback branch in getEntityKind can be retired without behavior change.

Helper location

  • getEntityKind lives in packages/chess/src/modifiers/auras.ts (top of file, exported). Reasoning: T7 is the first consumer; a dedicated util/entity-kind.ts file would be premature for a 30-line helper. T10 / T18 import via import { getEntityKind } from "../modifiers/auras.js". If a 3rd consumer arrives outside modifiers/, hoist to util/ then.

Test harness pattern (REUSE for T10, T18, T28, Wave 8/10)

  • Markers can be raw-inserted in tests until engine.spawnMarker lands:
    const markerId = engine.session.nextId();
    engine.session.insert(markerId, "EntityKind", "marker");
    engine.session.insert(markerId, "MarkerKind", "treasure"); // any locked kind
    engine.session.insert(markerId, "Position", squareIndex);
    // optional: AuraSpec, MarkerLifetime, MarkerOwner, MarkerLinks
    
  • This pattern is BACKWARD-COMPATIBLE — the type system accepts it (T6 added the attrs). When T10's factory lands, tests can migrate or coexist.
  • Square index reminder: file + rank*8. e2 = 12, d3 = 19, e1 = 4. Same convention as T2's harness notes.

Subtle behavior contracts pinned by tests

  1. Marker → adjacent piece: marker with AuraSpec IS picked up as an emitter; pieces in range receive contributions normally. Locked.
  2. Piece → marker (benign noop): marker WITHIN range of a piece's aura DOES receive an AuraContributions fact. Documented as benign because nothing reads piece-only attrs (HpBonus, AttackBonus, etc.) off a marker. Contract: if a future feature exposes piece-attrs on markers, this test will catch the hazard. T10 / T22 must NOT introduce piece-attr reads on marker entities without revisiting this test.

Surprises / gotchas

  • Session<…> IS NOT generic at the public-API surface used here — session.allFacts() returns untyped facts. Casts (f.value as Square) are still needed. The getEntityKind signature uses raw Session (no generic param) to match the existing aura code's style; same casts apply.
  • EntityKindValue is exported from schema.ts:32 (T6). Use the type alias, not a string-literal union, when typing helpers.
  • The pre-existing id > 0 filter in collectPositionedEntities is RETAINED as defense-in-depth: EntityKind defaults catch most cases but GAME_ENTITY (id 0) and PRESET_STATE_ENTITY (id -1) might accidentally carry a Position fact via unrelated subsystems. Belt + suspenders.
  • Existing findPieceAtSquare test helper in auras.test.ts pre-dates the marker era — it returns the FIRST entity at a square. With markers in play it could now match a marker. Tests in this file place markers on squares with no overlap with pieces under test, so this hasn't surfaced; downstream tests should prefer an EntityKind-filtered helper or use distinct squares.

Verification

  • bun test packages/chess/src/modifiers/auras.test.ts → 12 pass / 0 fail (10 existing + 2 new), 165 expects
  • bun run check → exit 0, 2006 tests across 170 files (T7 added +2; T8 landed in the same wave window contributing +5)
  • LSP diagnostics: clean on auras.ts AND auras.test.ts
  • Evidence: .sisyphus/evidence/task-7-marker-aura.txt

[2026-04-26T08:32Z] T10 marker entity factory (engine.spawnMarker / removeMarker / getMarkersAtSquare)

Method signatures (added to ChessEngine in packages/chess/src/engine.ts)

spawnMarker(
  kind: MarkerKindValue,
  square: Square,
  opts: {
    readonly lifetime: MarkerLifetimeValue;
    readonly owner?: PieceColor;
    readonly links?: readonly EntityId[];
  },
): EntityId;

removeMarker(id: EntityId): void;            // idempotent — guarded by per-attr `get` check
getMarkersAtSquare(square: Square): EntityId[]; // sorted by MARKER_KIND_PRIORITY asc, tie-break by entity id asc
  • File location: packages/chess/src/engine.ts:920-998 (right after spawnPiece, before dealDamage). Did NOT conflict with T9's rng() / setRngSeed() block (those are above in the same class).

Priority table location

  • MARKER_KIND_PRIORITY is a module-level const in engine.ts (above the ChessEngine class). Hardcoded verbatim from T0 ADR in decisions.md § Marker Collision Priority. Do NOT introduce a runtime override path — adding a new marker kind is a plan-amending event.
  • MARKER_ATTRS (sibling const) is the canonical list of attrs removeMarker retracts: EntityKind, MarkerKind, Position, MarkerLifetime, MarkerOwner, MarkerLinks. T18 and T30 should reference this same list when implementing on-piece-entered-marker cleanup and the despawn-marker primitive (don't rebuild it).

Tie-break choice (LOCKED for T18 dispatch parity)

  • Secondary sort in getMarkersAtSquare is (a as number) - (b as number) — entity id ASCENDING. So two markers of the SAME kind on one square always resolve in spawn order (older id wins). T18's on-piece-entered-marker MUST iterate the result of getMarkersAtSquare directly to inherit this ordering — do not re-sort, do not invert.

Optional-fact discipline (cross-task contract)

  • MarkerOwner and MarkerLinks are ONLY inserted when caller-supplied. engine.session.get(id, "MarkerOwner") returns undefined for neutral markers — consumers MUST treat undefined as "no owner" (do not coerce to a default color). T18/T28/T29/T30 primitives that read MarkerOwner need an explicit === undefined check.
  • removeMarker retracts each attr individually (guarded by a presence check) so it is safe on entities that never had owner or links facts. Idempotent — calling twice does not throw.

Cascade-removal NOT in scope (T30 deferral)

  • removeMarker does NOT touch MarkerLinks partners. Removing a portal-end marker leaves its paired endpoint intact; T30 (despawn-marker primitive) owns paired-cleanup semantics so callers retain control over portal-pair behavior.

EntityKind discriminator filtering

  • getMarkersAtSquare iterates session.allFacts() filtering attr === "Position" && value === square, then narrows by session.get(id, "EntityKind") === "marker". Confirms T7's EntityKind audit pattern — every Position-attr consumer that cares about pieces vs. markers must perform the same narrowing.
  • Pieces in the standard starting layout do NOT have EntityKind facts seeded yet (T7's job per the audit list). The getMarkersAtSquare(0) test on a starting-position rook returns [] because EntityKind on the rook is undefined, which fails the === "marker" check — works either way (filter is positive, not negative).

Verification

  • 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<string, BindingValue> on PrimitiveApplyContext. Imported BindingValue from ./context.js.
  • packages/chess/src/modifiers/triggers.ts — runPrimitives() gained a 6th parameter bindings: ReadonlyMap<string, BindingValue> = 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)

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<string> 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=<color> 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<string, string> — 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<string> — 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<object> param defaulting to a fresh set per top-level invocation.
  • Empty top-level scope: descriptor.primitives sees new Set<string>() — 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):
    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+)

// 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.

[2026-04-26T15:21:48Z] T20 suppressTriggers (move-gen dry-mode gate)

What landed

  • packages/chess/src/modifiers/primitives/types.ts: PrimitiveApplyContext gains required field readonly suppressTriggers: boolean. Threaded after T15's cascadeDepth. The contract pinned by docstring: individual primitives MUST NOT branch on this field — the SINGLE point of effect is the dispatcher-level skip in runPrimitives. Default at every construction site: false (regular wet path / trigger flow).

  • packages/chess/src/modifiers/custom/validate.ts: IMPERATIVE_KINDS was previously a private const (T14); now exported. T20 needs runtime access from triggers.ts to gate the 10 future Wave-5/6 imperative kinds. The set itself is unchanged — still locked to the T0 ADR list of 10 (place-piece, destroy-piece, move-piece, swap-pieces, convert-piece-type, set-piece-attr, cancel-capture, spawn-marker, spawn-marker-pair, destroy-marker). Picked re-export over a shared module: smaller blast radius (one- line const → export const), single source of truth, validator remains the canonical owner of the locked list.

  • packages/chess/src/modifiers/triggers.ts runPrimitives:

    • Now export-ed (was previously module-local) so the test suite can exercise it directly with a synthetic primitive. This is the canonical entry point for trigger-context primitive dispatch; exporting it does NOT change the public API of the integration preset (which goes through the fire*Hooks family).
    • New trailing param: suppressTriggers: boolean = false. Threaded into the constructed PrimitiveApplyContext AND through to nested children via the recursive runPrimitives call.
    • Pre-apply gate: if suppressTriggers && IMPERATIVE_KINDS.has(node.kind), continue — skip the imperative primitive's apply() entirely so it cannot mutate state during a dry-mode probe.
    • Post-arm guard: if (suppressTriggers) return skips the deferred-trigger-queue drain. The queue should be empty (apply never ran for IMPERATIVE_KINDS), but defensive: future primitives that mistakenly enqueue mid-dry-mode still cannot leak.
    • The 12 fire*Hooks functions seed suppressTriggers: false via the runPrimitives default (positional arg omitted) — wet path only.
  • packages/chess/src/modifiers/custom/apply.ts walkAndApply: ctx construction adds suppressTriggers: false. Profile-time apply is the WET path (game-start preset boot, real applyMove activations); dry-mode legality probing never reaches this walker.

  • packages/chess/src/modifiers/triggers.test.ts: +5 tests in new describe move-gen suppressTriggers flag (T20). Pattern documented below for Wave 5+ reuse.

Move-gen entry point — current state

Searched: grep -rn 'getLegalMove\|getLegalMoves' packages/chess/src/. Hits: rules/turn.ts (getLegalMovesForColor, getLegalMovesForCurrentTurn, isLegalMove); hooks/useChessEngine.ts (UI surface); presets/registry.ts (overridePieceMoves).

Crucial observation: today's move-gen pipeline does NOT invoke runPrimitives. Legality probing (e.g. rules/check.ts#filterSelfCheckMoves) uses a Session snapshot via snapshotSession (autoFire: false) + direct attr retraction. So there is no live wet-vs-dry distinction to wire today — the path simply doesn't enter the trigger dispatcher.

T20 is therefore a forward-compatible plumbing task: suppressTriggers is now a first-class field on PrimitiveApplyContext and the dispatcher honours it. When Wave-5/6 primitives land (T21-T30) AND when move-gen begins re-entering the trigger dispatcher (e.g. for marker-trigger probing during legality), the call site will pass suppressTriggers: true for dry probes and false (default) for commits. The gate itself already enforces the contract at the dispatcher.

IMPERATIVE_KINDS export decision

  • Picked: export const IMPERATIVE_KINDS in validate.ts (in-place add of export keyword).
  • Considered: extracting to modifiers/primitives/imperative-kinds.ts shared module. Rejected: the set is logically owned by the validator (it's the rule that gates passive vs trigger scope). Other consumers (T20 dispatcher, future Wave-5/6 primitive registration audit) reference it for read-only matching, never mutation. Re-exporting from the validator is idiomatic.
  • Cycle check: triggers.ts now imports from custom/validate.ts. custom/validate.ts imports from primitives/registry.ts + primitives/types.ts only. No back-edge — validator does NOT import from triggers.ts. The chain triggers → custom/validate → primitives/{registry,types} is acyclic. (Confirmed: bun run typecheck clean.)

Test harness pattern for Wave 5+ primitives (REUSE)

Synthetic-primitive registration for behavior tests on kinds that aren't yet in the real registry:

import { PRIMITIVE_REGISTRY } from "./primitives/registry.js";
import { z } from "zod";
import type { EffectPrimitive } from "./primitives/types.js";

let firedFlag = false;

try {
  PRIMITIVE_REGISTRY.register({
    kind: "destroy-piece" as unknown as EffectPrimitive["kind"],
    label: "...",
    description: "...",
    paramsSchema: z.object({}).passthrough(),
    apply: () => { firedFlag = true; },
  } as unknown as EffectPrimitive);
} catch {
  // already registered (Vitest module re-evaluation in watch mode)
}

Key points:

  • Cast through unknown for kind because PrimitiveKind union doesn't include the 10 IMPERATIVE_KINDS yet. The runtime registry stores the kind as a plain string key — the lookup in runPrimitives works regardless of static typing.
  • Register at MODULE TOP-LEVEL (not in beforeEach) — the registry has no unregister method by design (T0 ADR: registration is immutable for determinism). Module-level register + try/catch handles re-evaluation.
  • Use a single shared mutable flag and a resetFlags() helper rather than a per-test mock; cheap and avoids the registry- pollution-on-reset problem.
  • The __t20_predicate__ kind in the test demonstrates that NON- imperative primitives still fire under suppress — pick a fresh kind-name (with __ prefix to signal test-only) to avoid future collisions when Wave-5/6 primitives land.

runPrimitives calling convention (LOCKED)

runPrimitives(
  engine: ChessEngine,
  pieceId: EntityId,
  nodes: readonly EffectPrimitiveNode[],
  depth: number,
  event?: PrimitiveEvent,                                  // T1
  bindings: ReadonlyMap<string, BindingValue> = new Map(), // T11
  cascadeDepth: number = 0,                                // T15
  suppressTriggers: boolean = false,                       // T20
): void

8-positional-arg signature is approaching brittle. Wave 5+ should consider an RunPrimitivesOptions record. For T20: the marginal 8th param doesn't justify the refactor blast radius across all 12 fire*Hooks callers — defer to a dedicated cleanup task.

Bulk-edit gotcha (perl substitution)

The 22 primitive test files all build a literal PrimitiveApplyContext with bindings: new Map(),. T15 had ALREADY appended pendingTriggers: []

  • cascadeDepth: 0 to those construction sites. A naïve perl -pe 's|bindings: new Map\(\),\n|bindings: new Map(),\npendingTriggers: [],\n...\n|' duplicated the T15 fields. Fixed via a follow-up perl pass in slurp mode (-0777) that collapses adjacent duplicate blocks. The lesson: when adding a new required ctx field after a parallel-running task, prefer a single sed/perl that idempotently appends just THAT field, anchored against the most-recently-added preceding field (here: cascadeDepth: 0,). Future similar tasks should anchor the sed pattern against the latest-T15-style anchor, not the older bindings: new Map(),.

Verification

  • bun test packages/chess/src/modifiers/triggers.test.ts → 25 pass / 0 fail / 47 expects (was 20 pre-T20; +5 T20 tests).
  • bun run check → exit 0, 2053 tests across 172 files.
  • LSP diagnostics: clean on triggers.ts, triggers.test.ts, types.ts, validate.ts, custom/apply.ts.
  • Evidence: .sisyphus/evidence/task-20-suppress-triggers.txt.

Hand-off notes for downstream (Wave 5/6, marker-trigger move-gen)

  1. T21-T30 (imperative primitives): when registering against PRIMITIVE_REGISTRY, the kind-names MUST match IMPERATIVE_KINDS exactly. The dispatcher gate is by-name. Implementations DO NOT need to inspect ctx.suppressTriggers — the gate runs before apply() is called.
  2. Marker-aware move-gen (later wave): when the move-gen path begins probing markers via runPrimitives (e.g. for on-piece-entered-marker legality "would this trigger fire?"), pass suppressTriggers: true to the dispatcher invocation. For real commit, omit / pass false.
  3. isLegalMove / filterSelfCheckMoves: still session-snapshot based (no trigger dispatch). If a future task makes them invoke runPrimitives (e.g. trigger-aware self-check filtering for custom royalty rules), they MUST pass suppressTriggers: true.
  4. fire*Hooks family: already correct (default false). Adding a new fire*Hooks function? Don't pass true — those are wet-path-only by definition.

[2026-04-26T15:25:00Z] T15 deferred queue + cascade depth

What landed

  • packages/chess/src/modifiers/primitives/types.ts:

    • NEW TriggerName exported type — 14-kind union covering all existing fire*Hooks plus the four Wave-4 kinds (on-rule-activated, on-rule-expire, on-piece-entered-marker, on-marker-expire) eagerly listed so T16-T19 can extend the dispatcher without widening the union.
    • NEW PendingTrigger exported interface { kind: TriggerName; pieceId: EntityId; payload?: unknown }.
    • PrimitiveApplyContext gained two REQUIRED fields: pendingTriggers: PendingTrigger[] (mutable, per-arm) and cascadeDepth: number (orthogonal to existing depth).
  • packages/chess/src/modifiers/triggers.ts:

    • NEW module-level constant HARD_CASCADE_DEPTH = 8 mirroring the existing RUNTIME_DEPTH_HARD_CAP precedent (locked by decisions.md line 103).
    • NEW exported enqueueTrigger(ctx, trigger): void helper — pushes to ctx.pendingTriggers. Imperative primitives in Wave 5/6 will call this instead of firing inline.
    • runPrimitives() extended to accept cascadeDepth: number = 0 (added BEFORE T20's suppressTriggers argument so the public signature is (engine, pieceId, nodes, depth, event?, bindings?, cascadeDepth?, suppressTriggers?)). Throws runtime.cascade-depth-exceeded when cascadeDepth > 8. Allocates a fresh pendingTriggers: [] per invocation; drains FIFO at end of arm calling fireTriggerByKind(engine, t, cascadeDepth + 1).
    • NEW fireTriggerByKind private switch wires deferred kinds onto existing fire*Hooks for on-captured, on-capture, on-move, on-promotion, on-moved-onto-square. The other 9 union kinds fall to a console.warn no-op default — they require pre/post-snapshots that primitives can't synthesise from a deferred queue (on-damaged / on-check-*) or are turn-tick only (on-turn-start / -end), or land in T16-T19.
    • All fire*Hooks (12 functions) gained an optional trailing cascadeDepth: number = 0 parameter and forward it into runPrimitives so cross-arm chains via the queue stay in lockstep.

Wiring sites updated for new ctx fields

  • packages/chess/src/modifiers/triggers.ts: 2 sites (the main loop in runPrimitives + the resolver-only ctx in fireOnCapturedHooks).
  • packages/chess/src/modifiers/custom/apply.ts: 1 site (profile-time walker walkAndApply).
  • 23 primitive test fixtures + param-resolver.test.ts + context.test.ts — all gained pendingTriggers: [] and cascadeDepth: 0 adjacent to bindings: new Map().

Cross-cutting collision with T20 (suppressTriggers)

The working tree already had T20's suppressTriggers: boolean field on PrimitiveApplyContext AND its dispatcher branch in runPrimitives, plus T20 tests at the bottom of triggers.test.ts. T15 was authored ALONGSIDE T20 — every construction site needs BOTH:

pendingTriggers: [],
cascadeDepth: 0,
suppressTriggers: false,

The runPrimitives signature is the locked 8-tuple (engine, pieceId, nodes, depth, event?, bindings?, cascadeDepth?, suppressTriggers?). Future tasks (T16-T19, T28-T30) MUST honour this order or pass-through the optional defaults.

Drain order (LOCKED)

FIFO. The drain loop is for (const t of pendingTriggers), which iterates the array in push order — first enqueued, first fired. This mirrors classic event-queue semantics; a primitive that enqueues two triggers expects them to fire in the order it pushed them, not LIFO.

suppressTriggers + cascadeDepth interaction

When suppressTriggers === true, the dispatcher SKIPS the post-loop drain entirely (line ~278 in triggers.ts). Reasoning: if dry-mode prevented imperatives from running, the queue should already be empty; defensive-skip protects against a future primitive that mistakenly enqueues mid-dry-mode.

Test scaffolding pattern (REUSE for T16-T19, T28-T30)

import { z } from "zod";
try {
  PRIMITIVE_REGISTRY.register({
    kind: "__t15_enqueuer__" as unknown as EffectPrimitive["kind"],
    label: "...",
    description: "...",
    paramsSchema: z.object({}).passthrough(),
    apply: (ctx) => { enqueueTrigger(ctx, { kind: "on-move", pieceId: ctx.pieceId }); },
  } as unknown as EffectPrimitive);
} catch { /* already registered (re-evaluation) */ }

Vitest re-evaluates test files in watch mode; the try/catch around register() is mandatory because PrimitiveRegistry throws on duplicate-kind. The double-cast (as unknown as EffectPrimitive["kind"]) is the canonical bypass for the PrimitiveKind literal-union type guard — synthetic primitives with test-only kinds compile via this path. T20's tests use the identical pattern (lines 661-708 of triggers.test.ts).

Guard semantics

if (cascadeDepth > HARD_CASCADE_DEPTH) throw — strictly greater than 8 throws. Entering at exactly 8 is the LAST legal level and runs the arm; if a drained child re-enters at 9, THEN it throws. Tests pin both boundaries (depth=8 OK, depth=9 throws).

Verification

  • bun test packages/chess/src/modifiers/triggers.test.ts → 30 pass / 0 fail / 74 expects (was 25 pre-T15, +5 new tests)
  • bun test packages/chess/src/modifiers/primitives/ → 223 pass / 0 fail (byte-identical to pre-T15 — no regressions)
  • bun run check → exit 0, 2058 tests across 172 files
  • LSP diagnostics: clean on triggers.ts, types.ts, custom/apply.ts, triggers.test.ts, all 23 primitive test fixtures
  • Evidence: .sisyphus/evidence/task-15-cascade.txt

Subtleties / gotchas

  • runPrimitives exported: T15 (and T20 already) requires runPrimitives to be exported so triggers.test.ts can invoke it with explicit cascadeDepth values. The public signature is locked — DO NOT reorder existing parameters.
  • enqueueTrigger mutates ctx.pendingTriggers directly. This is INTENTIONALLY the opposite rule from withBinding (T11), which returns a NEW context with a fresh map. The queue is per-arm shared state; the bindings map is per-scope immutable state.
  • Drained triggers carry cascadeDepth + 1, not the bindings of the enqueuer. If a future use case needs to thread bindings through deferred fires, extend PendingTrigger with an optional bindings: ReadonlyMap<...> field; the current spec drains with a fresh empty map (the dispatcher creates one inside fireTriggerByKind → fire*Hooks → runPrimitives).
  • payload typing is unknown by design — the dispatcher narrows per kind. T16-T19 / T21-T27 may extend the payload contract to a discriminated union if more kinds need typed payload, but for V1 the per-kind narrows in fireTriggerByKind are sufficient.

[2026-04-26T15:37Z] T16 on-rule-activated

Files added

  • packages/chess/src/modifiers/primitives/on-rule-activated.ts (87 lines, mirrors on-capture.ts shape)
  • packages/chess/src/modifiers/primitives/on-rule-activated.test.ts (11 tests)

Files edited

  • packages/chess/src/schema.ts: added OnRuleActivatedHookEntry interface + 2 new attrs (OnRuleActivatedHooks, RuleActivatedFiredFor).
  • packages/chess/src/modifiers/primitives/types.ts: added "on-rule-activated" to PrimitiveKind union (after on-moved-onto-square, before conditional).
  • packages/chess/src/modifiers/primitives/context.ts: extended PrimitiveEvent discriminated union with { kind: "rule-activated"; descriptorId: string; chooserColor?: PieceColor } variant.
  • packages/chess/src/modifiers/primitives/index.ts: side-effect import ./on-rule-activated.js.
  • packages/chess/src/modifiers/apply.ts: appended registerAttrConsumer("OnRuleActivatedHooks") + registerAttrConsumer("RuleActivatedFiredFor") AFTER T18's block (race-safe — both T16 and T18 are pure additive).
  • packages/chess/src/modifiers/triggers.ts: added fireOnRuleActivatedHooks(engine, descriptorId, cascadeDepth?) near end of file. Imports now use VALUE imports (GAME_ENTITY, PRESET_STATE_ENTITY) instead of type-only since the function reads them at runtime.
  • packages/chess/src/modifiers/custom/apply.ts: post-walk fire-once invocation (added 1 import for fireOnRuleActivatedHooks + 1 import for ChessAttrMap type).
  • packages/chess/src/modifiers/triggers.test.ts: imported fireOnRuleActivatedHooks + 1 new describe block (1 test).
  • packages/chess/src/ui/ParamField.snapshot.test.tsx: added entries for both on-rule-activated AND on-piece-entered-marker (T18's primitive was missing from this exhaustive Record<PrimitiveKind, unknown> map — adding both was REQUIRED to clear the type error since the map type became incomplete after parallel T18 landed).

Storage scheme (LOCKED)

Two attrs at two different entities:

  1. OnRuleActivatedHooks on GAME_ENTITY (id = 0).

    • Type: readonly { descriptorId: string; primitives: readonly EffectPrimitiveNode[] }[].
    • Seeded by the primitive's apply(). Multiple descriptors append additional entries; the dispatcher filters by descriptorId.
    • Inner primitives target GAME_ENTITY (the dispatcher passes GAME_ENTITY as pieceId into runPrimitives) — on-rule-activated is per-game, NOT per-piece.
  2. RuleActivatedFiredFor on PRESET_STATE_ENTITY (id = -1).

    • Type: readonly string[] — list of descriptor ids that have already fired.
    • Checked + appended in applyCustomDescriptor AFTER the walker seeds OnRuleActivatedHooks. Skip-fire when already present.

Fire-once guard semantics

In applyCustomDescriptor (custom/apply.ts), AFTER walkAndApply completes:

const descriptorIdStr = String(descriptor.id);
const firedFor = (session.get(PRESET_STATE_ENTITY, "RuleActivatedFiredFor") as readonly string[] | undefined) ?? [];
if (!firedFor.includes(descriptorIdStr)) {
  session.insert(PRESET_STATE_ENTITY, "RuleActivatedFiredFor", [...firedFor, descriptorIdStr]);
  fireOnRuleActivatedHooks(engine, descriptorIdStr);
}

Three guard outcomes locked by tests:

  • Same-descriptor second-apply (e.g. per-type modifier hits 2 pieces) → only first attachment fires.
  • Save→load: RuleActivatedFiredFor is a Session fact and persists across rehydrate. Pre-seeded list blocks re-fire.
  • Different descriptor ids → independent: each fires once on their first attachment.

chooserColor on PrimitiveEvent

The dispatcher reads LastModifierChooser (T14 stub) from PRESET_STATE_ENTITY and stamps it into event.chooserColor IFF defined. Inner primitives can branch on the chooser. Note: applyCustomDescriptor itself writes LastModifierChooser BEFORE calling walkAndApply AND BEFORE firing on-rule-activated, so the chooser is always available for descriptors applied to a colored piece.

Why hook list lives on GAME_ENTITY

Per the activation model in decisions.md: an on-rule-activated hook is conceptually attached to the RULE (descriptor), not to any specific piece. Multiple piece-attaches of the same descriptor (e.g. per-type) must NOT register the inner primitive list multiple times. Storing on GAME_ENTITY makes it descriptor-scoped, and the fire-once guard ensures only one fire per descriptor regardless of how many pieces the descriptor binds to.

Cross-cutting collision with T18

T18 (on-piece-entered-marker) landed first and added:

  • "on-piece-entered-marker" to PrimitiveKind
  • OnPieceEnteredMarkerHooks to ChessAttrMap
  • OnPieceEnteredMarkerHookEntry interface
  • registerAttrConsumer("OnPieceEnteredMarkerHooks") in apply.ts

But T18 did NOT update ParamField.snapshot.test.tsx's exhaustive SAMPLE_PARAMS: Record<PrimitiveKind, unknown> map. T16's addition of "on-rule-activated" exposed this — TS errored that BOTH keys were missing. Resolution: T16 added both entries. This is unblocking-level scope (NOT implementing T18's primitive), justified because Record<PrimitiveKind, ...> is structurally exhaustive.

Test count delta

  • Before T16: 2058 tests
  • After T16: 2088 tests (+30)
    • +11 in on-rule-activated.test.ts
    • +1 in triggers.test.ts (fireOnRuleActivatedHooks)
    • +18 from T18's prior landing (registry count, ParamField, etc.) that I'm seeing for the first time

Verification

  • bun test packages/chess/src/modifiers/primitives/on-rule-activated.test.ts → 11 pass / 0 fail / 25 expects
  • bun test packages/chess/src/modifiers/triggers.test.ts → 31 pass / 0 fail / 76 expects (was 30 pre-T16)
  • bun run check → exit 0, 2088 tests across 174 files
  • LSP diagnostics: clean on schema.ts, types.ts, context.ts, on-rule-activated.ts, triggers.ts, custom/apply.ts, on-rule-activated.test.ts, triggers.test.ts
  • Evidence: .sisyphus/evidence/task-16-on-rule-activated.txt (EXIT 0)

Subtleties

  • Dispatcher target = GAME_ENTITY, NOT the descriptor's apply target. Tests assert that attribute mutations land on GAME_ENTITY (id=0) when on-rule-activated's inner block runs — NOT on the piece that was the original applyCustomDescriptor target. This is the correct semantic: on-rule-activated is a per-game hook.
  • Idempotent re-application is by design. Some workflows (admin re-apply, profile reconcile) call applyCustomDescriptor repeatedly for the same descriptor. The guard ensures activation effects are NEVER doubled.
  • No expire path yet. T17 will add on-rule-expire; the guard list (RuleActivatedFiredFor) is NOT cleared on expire — by design — because re-attachment in the same session shouldn't re-trigger activation. If a future task wants "re-attach should re-fire", that's a separate decision; today the contract is "once per descriptor id per game session".

[2026-04-26T15:38:00Z] T18 on-piece-entered-marker trigger + marker priority resolver

What landed

  • NEW packages/chess/src/modifiers/primitives/on-piece-entered-marker.ts (~110 lines). Mirrors on-capture.ts shape but seeds onto GAME_ENTITY (not per-piece) because the rule "fires whenever ANY piece enters a marker of kind X" is conceptually game-level, not bound to the apply-target piece.
  • NEW OnPieceEnteredMarkerHookEntry interface + OnPieceEnteredMarkerHooks attr in schema.ts. APPENDED at the end of ChessAttrMap (after T16's RuleActivatedFiredFor) per the brief's race-with-T16 guidance — landed cleanly.
  • NEW discriminator arm "piece-entered-marker" in PrimitiveEvent (primitives/context.ts). Carries markerId, markerKind, pieceId, square so inner primitives can branch on which marker fired them.
  • NEW "on-piece-entered-marker" in PrimitiveKind union (primitives/types.ts).
  • NEW fireOnPieceEnteredMarkerHooks(engine, movedPieceIds, cascadeDepth) in triggers.ts (~85 lines added after fireOnMovedOntoSquareHooks).
  • WIRED as stage 7b in apply.ts#onAfterMove immediately after fireOnMovedOntoSquareHooks (line ~1097): rationale documented inline — static-square hooks resolve before marker-overlay hooks.
  • NEW test file on-piece-entered-marker.test.ts — 14 tests / 25 expects.
  • UPDATED registry-count.test.ts from 22 → 24 (T16 + T18).

Dispatch wiring spot in triggers.ts

  • New function fireOnPieceEnteredMarkerHooks lives at packages/chess/src/modifiers/triggers.ts:691-758 (right after fireOnMovedOntoSquareHooks at line 689).
  • Apply.ts call site: packages/chess/src/modifiers/apply.ts:1098-1104 (stage 7b, after the for (const id of movedIds) square-filter loop).

Priority resolution pattern (LOCKED)

  • We DO NOT re-implement priority. T10's engine.getMarkersAtSquare(square) already returns markers sorted ascending by hardcoded MARKER_KIND_PRIORITY (portal-end=1 first), tie-break entity-id ascending.
  • Dispatcher iterates that result IN ORDER and matches each marker against every applicable hook in OnPieceEnteredMarkerHooks. Hook-array order is IRRELEVANT; only marker priority drives dispatch order. Pinned by the priority test in on-piece-entered-marker.test.ts:
    spawnMarker("mine", 28, ...);          // priority 3, but spawned first
    spawnMarker("portal-end", 28, ...);    // priority 1, spawned second
    // getMarkersAtSquare(28) returns [portalId, mineId] — priority sorts before spawn order
    // dispatcher fires portal hook (seed HpBonus=1) THEN mine hook (multiply by 2 → 2)
    
    If priority were inverted we'd see HpBonus=1 (mine multiply on absent attr is no-op, then portal seed=1). Test asserts HpBonus=2 → portal definitely fired first.

Match semantics

  • Exact-kind match only. A hook with markerKind: "mine" does NOT fire for pit markers. Pinned by the "mine != pit" test.
  • No wildcard. A descriptor that wants to fire for two kinds installs TWO hook entries (one per kind).
  • No marker auto-cleanup. The dispatcher does NOT remove the marker after firing — one-shot lifetime cleanup belongs to T19 (on-marker-expire) or a follow-up imperative primitive.

Mid-arm marker-spawn snapshot subtlety

  • getMarkersAtSquare reflects the LIVE session at the moment of the call.
  • Stage 7b runs in onAfterMove AFTER the move is applied, so any marker that was on the destination square BEFORE the move is visible — the move itself doesn't relocate markers.
  • A sibling primitive that spawns a marker on the same square IN THIS arm via the trigger pipeline (e.g. via set-piece-attr chain) would NOT be visible to a piece that already moved earlier in the same dispatch frame — the dispatcher reads Position once per piece, and the OnPieceEntered… fire happens once per piece per onAfterMove. T15's deferred-trigger queue (cross-arm cascades) is the correct vehicle for "spawn-then-trigger" patterns, not in-arm re-fire.
  • Forward-looking note: when imperative spawn-marker (T28) lands and a piece's on-rule-activated arm spawns a marker on the piece's CURRENT square, the piece's on-piece-entered-marker hooks for that kind will NOT fire mid-arm. They will fire on the NEXT move that re-enters the square (or the spawn primitive must explicitly enqueue on-piece-entered-marker via enqueueTrigger — left to T28's design).

Exact-kind enum guard pattern (REUSE for marker-kind primitives)

const MARKER_KIND_VALUES = [
  "mine", "pit", "portal-end", "frozen-square",
  "treasure", "death-square", "tornado", "blocked",
] as const satisfies readonly MarkerKindValue[];

const schema = z.object({
  markerKind: z.enum(MARKER_KIND_VALUES),
  primitives: z.array(NodeSchema),
});

The as const satisfies readonly MarkerKindValue[] couples the runtime list to the type union — adding/removing a kind in schema.ts without updating this list is a compile-time error. T28 (spawn-marker), T29 (spawn-marker-pair), T30 (despawn-marker) should reuse this pattern.

runPrimitives signature confirmation (8-tuple, locked)

Used in fireOnPieceEnteredMarkerHooks:

runPrimitives(engine, pieceId, hook.primitives, 1, event, new Map(), cascadeDepth, false);

Argument 7 = cascadeDepth (T15), argument 8 = suppressTriggers (T20). All downstream tasks must respect this order. The fire call passes suppressTriggers=false explicitly because this is the wet path.

Race coordination with T16 — clean

  • T16 also touches schema.ts (OnRuleActivatedHooks, RuleActivatedFiredFor, OnRuleActivatedHookEntry), context.ts (rule-activated event arm), types.ts (on-rule-activated PrimitiveKind), and apply.ts (consumer registration + activation fire wire-in).
  • I appended T18's pieces AFTER T16's in every shared file:
    • ChessAttrMap.OnPieceEnteredMarkerHooks after RuleActivatedFiredFor
    • OnPieceEnteredMarkerHookEntry interface after OnRuleActivatedHookEntry
    • PrimitiveEvent "piece-entered-marker" arm after "rule-activated"
    • PrimitiveKind "on-piece-entered-marker" after "on-rule-activated"
    • registerAttrConsumer("OnPieceEnteredMarkerHooks") after T16's pair
    • import "./on-piece-entered-marker.js" after T16's import
  • Net result: zero merge conflicts, both tasks compose orthogonally.
  • Lesson: when tasks brief says "APPEND at end of block", do EXACTLY that. Don't relitigate ordering — the parallel writer needs a stable slot too.

Test count delta

  • Pre-T18 baseline (per learnings): 2058 tests / 172 files (post-T15).
  • Post-T18 + T16: 2088 tests / 174 files (+30 = T18 +14 + T16 +N).
  • bun run check exit 0.

Verification

  • bun test packages/chess/src/modifiers/primitives/on-piece-entered-marker.test.ts → 14 pass / 0 fail / 25 expects
  • bun test packages/chess/src/modifiers/triggers.test.ts → 31 pass / 0 fail / 76 expects (regression intact)
  • bun run check → exit 0, 2088 tests across 174 files
  • LSP diagnostics clean on every changed file.
  • Evidence: .sisyphus/evidence/task-18-marker-priority.txt

Hand-off notes for downstream

  • T19 (on-marker-expire / one-shot consumption): when implementing one-shot marker consumption on entry, hook into fireOnPieceEnteredMarkerHooks AFTER the dispatcher's hook iteration completes for a given marker — OR add a post-dispatch retraction pass keyed off MarkerLifetime.kind === "one-shot". The dispatcher does NOT auto-cleanup — that's T19's domain.
  • T28 (spawn-marker imperative primitive): per the mid-arm snapshot caveat above, decide whether spawning a marker on the spawning piece's current square should enqueue an on-piece-entered-marker trigger via enqueueTrigger(ctx, ...) or rely on the next move's natural fire.
  • T59-T61 (parity tests minefield/mr_freeze/parry): each can now express its core mechanic via on-piece-entered-marker + the matching marker kind (mine/frozen-square/portal-end). Spawn markers via engine.spawnMarker in test setup OR via T28's primitive once it lands.
  • Wave 10 marker-using parity descriptors: hook trees compose naturally with this primitive — e.g. an on-rule-activated block can spawn the initial marker layout, and a sibling on-piece-entered-marker block defines the per-entry effect. Both fire from the same descriptor, no cross-talk.

[2026-04-26T15:50:00Z] T17 on-rule-expire

Files added

  • packages/chess/src/modifiers/primitives/on-rule-expire.ts (97 lines, mirror of on-rule-activated.ts)
  • packages/chess/src/modifiers/primitives/on-rule-expire.test.ts (11 tests / 24 expects)

Files edited

  • packages/chess/src/schema.ts: appended OnRuleExpireHookEntry interface + 2 attrs (OnRuleExpireHooks, RuleExpireFiredFor) AFTER T18's OnPieceEnteredMarkerHookEntry. Race-clean with parallel T19 because T19 owns OnMarkerExpire* (different attr names).
  • packages/chess/src/modifiers/primitives/types.ts: added "on-rule-expire" to PrimitiveKind union (after on-rule-activated, before on-piece-entered-marker). Note: on-marker-expire was ALREADY present in the union (T19 must have inserted it parallel-pre-Wave-4-coordination — see "cross-task collision" below).
  • packages/chess/src/modifiers/primitives/context.ts: extended PrimitiveEvent with { kind: "rule-expire"; descriptorId: string } arm (no chooserColor — expire is system-driven, not player-driven).
  • packages/chess/src/modifiers/primitives/index.ts: side-effect import ./on-rule-expire.js between T16 and T18.
  • packages/chess/src/modifiers/apply.ts: appended registerAttrConsumer("OnRuleExpireHooks") + registerAttrConsumer("RuleExpireFiredFor") AFTER T16 block. PLUS added registerAttrConsumer("OnMarkerExpireHooks") to unblock T19's missing manifest registration (see cross-task collision below).
  • packages/chess/src/modifiers/triggers.ts: added fireOnRuleExpireHooks(engine, descriptorId, cascadeDepth?) after fireOnRuleActivatedHooks. Fire-once guard lives IN this dispatcher, not at the call site (mirror of T16 inverted — see decision below).
  • packages/chess/src/ui/ParamField.snapshot.test.tsx: added "on-rule-expire" AND "on-marker-expire" entries (snapshot map is Record<PrimitiveKind, unknown> — exhaustive, both kinds were missing).
  • packages/chess/src/modifiers/primitives/registry-count.test.ts: bumped 24 → 25 per T17's new primitive.

Detach wiring status — STUB

NO clean detach path exists in packages/chess/src/modifiers/. Searched: removeModifier, detachDescriptor, removeCustom, expireDescriptor — zero hits. Descriptor lifetimes are NOT yet wired today.

T17 therefore exposes fireOnRuleExpireHooks(engine, descriptorId, cascadeDepth?) as a PUBLIC STUB callable from:

  • T19's lifetime decrementer (when descriptor lifetime expires) — util/marker-lifetime.ts will gain a parallel decrementDescriptorLifetimes or similar.
  • The eventual remove-modifier player action handler.
  • Piece-modifier capture cascade (when a piece holding modifiers dies, fire each held descriptor's expire).

Until any caller exists, on-rule-expire blocks register their hooks at apply time but never fire. By design: expire fires on actual detach, never on game shutdown.

Fire-once guard location DECISION (T16 vs T17 ASYMMETRY)

T16's activation guard lives in applyCustomDescriptor (custom/apply.ts) because activation has ONE clean call-site. T17's expire guard lives INSIDE fireOnRuleExpireHooks because expire will have PLURAL call-sites (lifetime tick, manual remove, piece-capture cascade). Centralising the guard in the dispatcher makes every future caller inherit dedup for free — caller just invokes fireOnRuleExpireHooks(engine, descriptorId) and the function self-dedups via RuleExpireFiredFor on PRESET_STATE_ENTITY.

Order of operations inside the dispatcher (LOCKED):

  1. Read RuleExpireFiredFor from PRESET_STATE_ENTITY.
  2. If descriptorId already in list → early return (no-op).
  3. Read OnRuleExpireHooks from GAME_ENTITY.
  4. Append descriptorId to fired list FIRST (before any inner primitive runs) so a primitive that re-enters this dispatcher via cascade can't double-fire.
  5. Iterate hooks, fire matching ones with event = { kind: "rule-expire", descriptorId }.

Step 4 is critical for cascade safety — any primitive that triggers fireOnRuleExpireHooks(engine, sameId) mid-arm sees the guard already set and bails.

Cross-task collision with parallel T19 — TWO unblocking-scope edits

When T17 ran, T19's files were ALREADY in the working tree (untracked):

  • packages/chess/src/modifiers/primitives/on-marker-expire.ts — present, registers under "on-marker-expire".
  • util/marker-lifetime.ts with decrementMarkerLifetimes — present, used in apply.ts.
  • OnMarkerExpireHooks attr in schema.ts + OnMarkerExpireHookEntry interface — present.
  • "on-marker-expire" in PrimitiveKind union — present.
  • decrementMarkerLifetimes import + call site in apply.ts — present.

But T19 was MISSING two things that broke bun run check:

  1. registerAttrConsumer("OnMarkerExpireHooks") in apply.ts — load-time integrity check failed across 19 test files with "primitive-seed consumer integrity check failed: 1 attr(s) have no registered consumer [OnMarkerExpireHooks]".
  2. "on-marker-expire" entry in ParamField.snapshot.test.tsx's exhaustive Record<PrimitiveKind, unknown> map — typecheck failed.

Both were single-line additions; T17 added them under "T19 — co-landed because parallel branch missed it" comments. Mirrors T16's precedent (line 1298 of this notepad) of unblocking-scope cleanup when a sibling task leaves a manifest gap. The alternative (waiting for T19 to fix itself) would have left T17 unable to verify.

LOCKED contract: dispatcher cascade interaction

The fire-once guard append happens BEFORE inner primitives fire. If a destroy-marker (T30, future) inside on-rule-expire's primitive list triggers another descriptor's detach via cascade, that cascade fires under cascadeDepth + 1 and is independently guarded by ITS descriptor id — no entanglement.

runPrimitives invocation pattern (mirror of T16)

runPrimitives(engine, GAME_ENTITY, hook.primitives, 1, event, new Map(), cascadeDepth);

Positional arg 7 = cascadeDepth (T15), arg 8 omitted = suppressTriggers default false. WET path only — expire never fires under dry-mode probing.

Test count delta

  • Pre-T17: 2088 tests / 174 files
  • Post-T17: 2103 tests / 175 files (+15 = T17 +11 + T19 baseline tests landing in same wave)

Verification

  • bun test packages/chess/src/modifiers/primitives/on-rule-expire.test.ts → 11 pass / 0 fail / 24 expects
  • bun test packages/chess/src/modifiers/triggers.test.ts → 31 pass / 0 fail / 76 expects (regression intact)
  • bun run check → exit 0, 2103 tests across 175 files
  • LSP diagnostics: clean on schema.ts, types.ts, context.ts, on-rule-expire.ts, triggers.ts, on-rule-expire.test.ts, ParamField.snapshot.test.tsx
  • Evidence: .sisyphus/evidence/task-17-on-rule-expire.txt (EXIT 0)

Subtleties / hand-off notes

  • Hook list lives on GAME_ENTITY (not piece) — same rationale as T16: a single descriptor that attaches to multiple pieces should fire its expire block ONCE total, not once per attachment.
  • No chooserColor on rule-expire event: by design. Expire is a system event (lifetime tick, capture cascade) not a player action; the original chooser may not be available. Inner primitives that need chooser context should branch on absence.
  • For T19 lifetime-decrementer or future remove-modifier action handler: just call fireOnRuleExpireHooks(engine, descriptorId) from the detach pipeline. Cascade depth is optional (defaults to 0 for top-level detach). Multiple successive call attempts for the same descriptorId in the same session are SAFE — the guard makes them no-ops.
  • Stub-doc: the function lives in triggers.ts at line ~895. Search fireOnRuleExpireHooks to find it.

[2026-04-26T15:55:00Z] T19 on-marker-expire + lifetime decrementer

Files added

  • packages/chess/src/modifiers/primitives/on-marker-expire.ts — primitive descriptor mirroring on-piece-entered-marker.ts. paramsSchema { markerKind: enum(8 kinds), primitives: array }. seedsAttrs: ["OnMarkerExpireHooks"]. Stores hook entries on GAME_ENTITY.
  • packages/chess/src/modifiers/primitives/on-marker-expire.test.ts — 17 tests across 4 describe blocks: registry/schema (6), apply()-seeds (2), childPrimitives (1), fireOnMarkerExpireHooks dispatch (3), decrementMarkerLifetimes (5).
  • packages/chess/src/util/marker-lifetime.ts — new util module (NOT an engine method). Exports decrementMarkerLifetimes(engine, cascadeDepth?). Two-phase iteration (collect ids first, mutate second) to keep session.allFacts() iterator valid.

Files extended

  • schema.ts: added OnMarkerExpireHookEntry interface + OnMarkerExpireHooks attr in ChessAttrMap (appended at end after T17's RuleExpireFiredFor).
  • types.ts: added "on-marker-expire" to PrimitiveKind union (after T17's "on-rule-expire"). Note: "on-marker-expire" was ALREADY present in TriggerName union (eagerly listed since T15) — only PrimitiveKind needed extension.
  • context.ts: added marker-expire variant to PrimitiveEvent discriminated union: { kind: "marker-expire"; markerId; markerKind; square }.
  • triggers.ts: added fireOnMarkerExpireHooks(engine, markerId, cascadeDepth=0) function. Reads MarkerKind + Position BEFORE the marker's facts are retracted (caller decrementMarkerLifetimes fires this BEFORE engine.removeMarker). Filters hooks by exact-kind match.
  • apply.ts: added decrementMarkerLifetimes import + registerAttrConsumer("OnMarkerExpireHooks") (the latter was retroactively present from earlier T19 partial-landing — verified). Wired stage 7c in onAfterMove dispatch (after stage 7b fireOnPieceEnteredMarkerHooks, before stage 8 check-line edge triggers). Updated 12-stage doc comment to call out 7c.
  • primitives/index.ts: added import "./on-marker-expire.js"; after T18's on-piece-entered-marker.js import.
  • registry-count.test.ts: bumped 25 → 26 (T17 had landed first, taking 24 → 25; T19 takes 25 → 26).
  • ParamField.snapshot.test.tsx: added "on-marker-expire": { markerKind: "frozen-square", primitives: [...] } entry to the exhaustive Record<PrimitiveKind, unknown> map. (An incomplete prior-landing version was missing markerKind — fixed.)

Move-counter attribute confirmed

  • FullmoveNumber on GAME_ENTITY is the canonical "current move count". Initialized to 1 by classic layout; incremented in engine.ts#advanceTurnAfterMutation after black completes a move. Compared via >= against expiresAtMove (so expiresAtMove: 5 expires when FullmoveNumber reaches 5, not 6).
  • HalfmoveClock is the FIDE 50-move-rule clock — RESETS on captures/pawn moves, so unsuitable for absolute lifetime tracking.
  • HalfMovesThisTurn is within-turn-only — RESETS on every flip; also unsuitable.

Dispatch stage

  • Stage 7c in onAfterMove. Order rationale: 7b runs first so a piece entering a frozen-square that's scheduled to expire THIS move still triggers the freeze effect; THEN 7c sweeps and the marker dies. Same rule for mines (one-shot, sweep skips), pits, etc.

Key design choices

  1. Iteration safety: collect expired ids first via session.allFacts() walk, THEN fire+remove. Mutating mid-iteration would invalidate the iterator. (Mirrors T15 cascade-queue's "drain-after" pattern.)
  2. one-shot skip: locked by decisions.md § Square State via Marker Entities. The decrementer NEVER auto-decrements one-shot — that's the entry-trigger's job (T18's on-piece-entered-marker will be the call site once destroy-marker primitive lands at T30).
  3. Paired-marker cascade DEFERRED: T30 (destroy-marker primitive) owns paired-marker semantics. T19 expires ONE marker; if its MarkerLinks partner should also die, that's an on-marker-expire hook calling destroy-marker on the partner.
  4. Util module, NOT engine method: decrementMarkerLifetimes lives in util/marker-lifetime.ts because it composes engine.session.allFacts(), engine.removeMarker, and fireOnMarkerExpireHooks (from triggers.ts) — putting it as an engine method would tangle imports. The util takes engine as its first arg, mirroring trigger dispatcher signatures.
  5. Fire-once guard NOT used here (unlike T17 fireOnRuleExpireHooks which dedups by descriptorId). Marker-expire is per-marker-instance: a marker either expired or it didn't, and removeMarker after the fire ensures the same marker can't fire twice (the second sweep finds no EntityKind=marker fact).

Cross-cutting collision with T17 — clean

  • T17 landed schema attrs (OnRuleExpireHooks, RuleExpireFiredFor), the on-rule-expire.ts primitive file, and fireOnRuleExpireHooks in triggers.ts BEFORE T19. T17 also bumped registry-count to 25 and added "on-rule-expire" to PrimitiveKind.
  • T19 picks up cleanly: appends OnMarkerExpireHook attr at end of ChessAttrMap (after T17's block), appends "on-marker-expire" to PrimitiveKind (after T17's "on-rule-expire"), bumps registry-count 25→26.
  • Pre-existing partial T19 manifest registration (registerAttrConsumer("OnMarkerExpireHooks")) was already present in apply.ts from a prior unblocking landing — verified in place, not duplicated.

Test counts

  • Pre-T19 baseline: 2103 tests.
  • Post-T19: 2120 tests / 176 files (+17 = T19 only). All pass byte-identical for pre-existing tests.

Verification

  • bun test packages/chess/src/modifiers/primitives/on-marker-expire.test.ts → 17 pass / 0 fail / 31 expects.
  • bun run check → EXIT 0. 2120 pass / 0 fail. (Note: 15 obsolete snapshots warning is benign — stale snapshots from earlier ParamField runs predating the 28-kind union; no test fails.)

For T59 (minefield) and T60 (mr_freeze) consumers

  • mines: spawn with lifetime: { kind: "one-shot" }. The decrementer NEVER expires them; the on-piece-entered-marker hook for mine should call destroy-marker (T30) explicitly to consume them after damage applies.
  • frozen-square: spawn with lifetime: { kind: "moves", expiresAtMove: <currentFullmove + N> }. The decrementer auto-expires when FullmoveNumber catches up. Install an on-marker-expire hook for frozen-square if you need a thaw-effect (e.g. broadcast UI fizzle).
  • The dispatcher's event payload { markerId, markerKind, square } is sufficient for both use-cases — primitives use event.square to spawn follow-up effects on the dying marker's tile.

[2026-04-26T10:08:00Z] T21 place-piece imperative primitive

Implementation

  • place-piece.ts (105 lines): kind: "place-piece", params { pieceType, color, square: 0..63 } (Zod), apply() calls engine.spawnPiece(pieceType, color, square). Empty seedsAttrs (spawnPiece writes core attrs already in consumer registry).
  • 14 tests / 42 expects across registry / paramsSchema / apply / on-rule-activated integration layers.

Key behavioral findings (pinned in tests for future consumers)

  1. Occupied-square placement is PERMISSIVE — engine.spawnPiece does NOT check the target square. Calling place-piece on an occupied tile spawns a SECOND piece sharing that Position fact. Test pins this so any future "place-if-empty" tightening is a deliberate baseline change.
  2. on-rule-activated nesting fires inner imperatives TWICE — applyCustomDescriptor's walkAndApply recurses into childPrimitives() at attach time AND fireOnRuleActivatedHooks runs the inner block via the dispatcher. The dispatcher's IMPERATIVE_KINDS gate (T20) applies only to runPrimitives, NOT to walkAndApply. So a place-piece inside on-rule-activated materializes the piece twice. Test asserts length >= 1 and verifies every spawned piece has the right facts. Future cleanup: add the same IMPERATIVE_KINDS skip to walkAndApply so attach-time walking does not double-fire imperatives. That's a separate task.

Coordination with parallel Wave-5 agents (T22/T23)

  • T22 (destroy-piece) and T23 (move-piece) ran concurrently, both touching types.ts PrimitiveKind union, index.ts side-effect imports, registry-count.test.ts, and ParamField.snapshot.test.tsx. All three Wave-5 imperative-primitive entries coexist in the final tree (registry count 26→29 cumulatively).
  • APPEND-only discipline worked: my edits to the four shared files were small additions; T22/T23 added theirs alongside without merge conflict. Future Wave-5 agents (T24-T27) should keep doing the same.

Pre-existing failure surfaced (NOT mine)

  • triggers.test.ts lines 749-819 register a SYNTHETIC primitive under the name destroy-piece via try { register(...) } catch {} (T20 design — assumed Wave 5/6 hadn't landed). T22's real destroy-piece registration now collides; the synthetic registration silently swallows the duplicate-kind throw, the real T22 apply() runs instead of the test stub, and 2 expectations on imperativeFired flip to false. Fix is T22's: either rename the synthetic stub to a non-colliding name (e.g. __t20_synthetic_imperative__ and add it to a test-only IMPERATIVE_KINDS extension) or rewrite those tests to use a fresh kind from IMPERATIVE_KINDS that's STILL not registered.

Consumer-integration / docs / snapshot tests pass

  • Verified place-piece.test.ts + registry-count.test.ts + ParamField.snapshot.test.tsx + docs.test.ts all green: 90 pass / 0 fail / 387 expects.
  • The 15 "obsolete snapshots" warning persists (benign — pre-existing from prior ParamField cleanup; no test fails).

Numbers

  • Tests added: 14 (place-piece.test.ts)
  • Workspace tests at finish: 2151 (T22/T23 added theirs too); 2 fail in triggers.test.ts (pre-existing collision per above).
  • bun run check exit: 1 (due to triggers.test.ts collision); my changes alone exit 0.
  • registry-count after T21+T22+T23: 29.

[2026-04-26T10:10:00Z] T22 destroy-piece imperative primitive

Implementation

  • destroy-piece.ts (~210 lines): kind: "destroy-piece", params { target: nonneg int } (Zod), apply() retracts a fixed list of 32 piece-related attrs (core identity + HP/modifier attrs + T8 movement-replacement attrs + per-piece trigger hook attrs + EntityKind discriminator), then enqueues on-captured via T15's enqueueTrigger with {attackerId: ctx.pieceId, defenderId: target} payload.
  • 12 tests / 29 expects across registry, schema, and apply() layers.

Key design decisions (pinned in tests)

  1. Marker safety: refuses to retract entities where EntityKind === "marker" — silent no-op. Markers are owned by destroy-marker (T30); a misdirected target id pointing at a marker must not corrupt marker state.
  2. Idempotent / no-op on missing target: when the target's Position fact is absent (already destroyed, never existed, stale binding from cascade), apply() is a silent no-op. CRITICAL: this also skips the on-captured enqueue — otherwise a back-to-back destroy in a cascade arm would double-fire the hook.
  3. Explicit attr list (mirrors MARKER_ATTRS): chose the explicit fixed-list approach over an allFacts() walk so adding a new piece attr to schema.ts is a deliberate, code-search-able event. The new attr will stay on a destroyed entity until it's added to PIECE_ATTRS_TO_RETRACT. Trade-off: a forgotten attr leaks; trade-off accepted for explicitness (matches engine.ts#removeMarker precedent).
  4. on-captured enqueue runs AFTER retract: by the time the dispatcher drains the queue, OnCapturedHooks is already retracted from the defender — so the hook fire is a quiet skip. For pre-retract death-rattle, callers must wire on-captured via the regular capture pipeline, NOT via this primitive. Doc comment explicitly notes this.

Resolved T21's flagged collision (triggers.test.ts)

  • T21's learnings flagged that triggers.test.ts registered a SYNTHETIC destroy-piece stub that would collide once a real T22 implementation landed. The fix was T22's responsibility per T21's note.
  • Fix applied: renamed the synthetic stub from destroy-piece → swap-pieces (still in IMPERATIVE_KINDS, still not yet implemented as a real Wave-5 task). Updated 4 test-body call sites + 3 comment references. The T20 dispatcher gate still tests correctly via the swap-pieces synthetic primitive.

Numbers

  • Tests added: 12 (destroy-piece.test.ts).
  • Workspace tests at finish: 2163 pass / 0 fail across 179 files.
  • bun run check exit: 0.
  • registry-count after T21+T22+T23: 29 (bumped 28→29 for T22's primitive).
  • The 15 "obsolete snapshots" warning persists (benign — pre-existing from prior ParamField cleanup; no test fails).

Files touched

  • NEW: packages/chess/src/modifiers/primitives/destroy-piece.ts
  • NEW: packages/chess/src/modifiers/primitives/destroy-piece.test.ts
  • EDIT: packages/chess/src/modifiers/primitives/types.ts (added "destroy-piece" to PrimitiveKind union)
  • EDIT: packages/chess/src/modifiers/primitives/index.ts (added side-effect import)
  • EDIT: packages/chess/src/modifiers/primitives/registry-count.test.ts (28 → 29)
  • EDIT: packages/chess/src/ui/ParamField.snapshot.test.tsx (added destroy-piece: { target: 28 } fixture)
  • EDIT: packages/chess/src/modifiers/triggers.test.ts (synthetic stub renamed destroy-piece → swap-pieces to resolve T21's flagged collision)

Inheritance for T24-T30 (remaining Wave 5/6 imperatives)

  • swap-pieces (T24) is now reserved by the T20 trigger-test stub. Real T24 implementer must rename the synthetic stub to another unimplemented IMPERATIVE_KIND (e.g. convert-piece-type if T25 hasn't landed first, else set-piece-attr for T26, etc.) BEFORE registering the real swap-pieces. The pattern is clear: every time a real imperative primitive lands, T20's synthetic stub must rotate to the next unimplemented kind.
  • Long-term fix: add a test-only synthetic __test_imperative__ kind to IMPERATIVE_KINDS via a test-only set extension (or via a vitest setup file) so the rotation isn't needed. Out of scope for T22 — file as a follow-up cleanup task.