From 3b2fbdbc2b253443f432ef214de8dd08dfc5d450 Mon Sep 17 00:00:00 2001 From: lgray Date: Mon, 3 Aug 2026 20:28:10 -0500 Subject: [PATCH 1/3] fix(engine): fuse battlefield-entry record+emit into one authority Six sibling sites emitted a `ZoneChanged { from: None, to: Battlefield }` event carrying a placeholder `turn_zone_change_index` of `0`. The batched-trigger collector dedups on `(definition_ref, turn_zone_change_index)` read off the event, so a later same-turn entry aliased onto the first occurrence's key, was treated as already-collected, and its batched enters-the-battlefield fire was swallowed. Measured on the pre-fix tree: two same-turn conjures produce dedup keys `[0]` and one trigger fire; post-fix they produce `[0, 1]` and two fires. Within a single event the placeholder was additionally wrong in a second way: every entrant recorded index `0`, so `self_ref_own_departure_successor` subscripted another object's ledger row. That case is fixed here too (`[0, 0]` -> `[0, 1]`), though it is not the case that lost a trigger. CR 603.2c: an ability triggers once per occurrence, and repeatedly when one event contains multiple occurrences. CR 603.6a: each time an event puts one or more permanents onto the battlefield, all permanents are checked for matching ETB triggers. Adds `zones::record_and_emit_entry_from_no_zone`, which fuses the snapshot, the `record_zone_change` ledger write (which performs the CR 608.2i battlefield-entry bookkeeping itself) and the event emission into one indivisible call. The defect class is literally "emit without record", so fusing is what narrows it: a record-only helper plus a separate emit helper is today's shape, and today's shape is what produced the defect. Sites routed: conjure, counters (InjectPredefinedTokenAbilities + both FinalizeTokenEntry / FinalizeCopyTokenEntry arms), token_copy x2, gift_delivery, incubate. Deletes the private clone `counters::push_token_entry_events` and both `token.rs` halves (`record_committed_token_entry`, `push_token_entry_events_for_record`), folding them into the authority; `push_committed_token_entry_events` now returns `Option`. That return is consumed at exactly two of its eight call sites -- `gift_delivery.rs:175` and `token_copy.rs:866`, both `.expect(...)` (`rg -n -A8 'push_committed_token_entry_events\(' crates/engine/src`) -- which preserves the panic those routes already had when a just-created token is missing; the flush route discards it. That sentence is about THIS emitter. It does not describe `zones::record_and_emit_entry_from_no_zone` one level down, whose own four direct callers are `conjure.rs:218` and `incubate.rs:123` (both `.expect`, i.e. they PANIC on `None`), `token.rs:1881` (`is_some()`), and `counters.rs:530` (discards). An earlier revision copied this sentence into that function's doc, where it named the wrong callers and framed the `None` arm as inert everywhere; that doc is corrected here and now carries its own query. The object-gone arm is load-bearing through its EFFECT (nothing recorded, nothing emitted), not through the returned value. Each site's co-located `record_battlefield_entry` call is deleted in the same edit, because `record_zone_change` already performs it and leaving both double-counts `battlefield_entries_this_turn`. The invariant is structural, not enumerated: `TokenCreated` is emitted IF AND ONLY IF the authority recorded the entry. `push_committed_token_entry_events` gates the push on `record.is_some()`, and `record_and_emit_entry_from_no_zone` returns `None` exactly when the object is gone -- verified to have exactly one `?`, on `state.objects.get`, with no second `None` source -- so the authority's own verdict is the predicate. `restrictions::record_token_created` applies that same existence test to `created_tokens_this_turn` and `players_who_created_token_this_turn`, so event and ledger cannot disagree on any route THAT REACHES THIS EMITTER -- which is the class boundary, stated because the measurement was made against it. Two routes do not reach it: `incubate.rs` and `counters.rs`'s `InjectPredefinedTokenAbilities` arm call `record_token_created` and then the authority DIRECTLY, so they write both ledgers and emit no `TokenCreated` at all. That is pre-existing and unchanged here -- `b654513cb` behaves identically -- but it means Incubator tokens do not fire `TriggerMode::TokenCreated`, even though CR 701.53a describes incubating as creating a token. Routing them through this emitter would reorder the ETB events the `InjectPredefinedTokenAbilities` arm exists to control, so it is recorded as follow-up rather than done here. A token creation writes THREE ledgers, not two, and gating the event on the authority's verdict at first only moved which one it disagreed with. The third is `last_created_token_ids`, the `TargetFilter::LastCreated` anaphora slot (`game/filter.rs`'s `contains(&object_id)`, `game/targeting.rs`'s `clone()`). On the object-gone path it produced `(TokenCreated=0, created_tokens_this_turn=0, last_created_token_ids=1)`: a "the token you created" reference naming an object that never finished entering. Adds `token::record_last_created_token`, which carries the same existence predicate. That population is stated WITH the query that produced it AND with the tip it was run at, because this ledger has now been mis-swept twice from a too-narrow grep and mis-STATED twice from an unlabelled tip. Counts are ripgrep JSON MATCH EVENTS (`rg --pcre2 -U --json ... | grep -c '"type":"match"'`). An event is a matched CHUNK and ripgrep emits one per LINE, so a line carrying two matches counts once. Both populations below are unaffected -- `--count-matches` sums to the same 39 and 11 -- but two positive CONTROLS quoted later in this message ARE affected, and both figures are given where they appear. With `M=(push|extend|clear|insert|append|retain|splice|truncate|remove|drain|resize|pop)`, `rg -n --pcre2 -U "state\s*\.\s*last_created_token_ids\s*(\.\s*M\s*\(|=[^=])" crates/engine/src` returns, RUN AT THIS COMMIT, 39 hits, of which 3 are prose in comments this commit itself added; the remaining 36 code hits plus one more bound as `s.` (`engine.rs`'s per-turn `.clear()`) are 37 = 20 production writers and 17 inside `#[cfg(test)]`. Of those 20, exactly ONE publishes a single just-created id -- the `push` inside `record_last_created_token` -- and the other 19 are 4 clears and 15 bulk republishes. RUN AT `b654513cb`, the same query returns 39 hits with NO comment among them, for 23 production writers, of which FIVE published the id of the single token the caller had just created. Those five are now the authority's five production CALL SITES (`counters.rs` x3, `token_copy.rs`, and the liminal finalizer in `token.rs`) -- calls, not writers, which is why they are absent from this commit's 20. An earlier revision of this message stated that BASE-tip call-site count of 5 in the present tense against the HEAD-tip writer population of 20, and partitioned 20 as 5 + 15, which omits the clears and contradicts this commit's own `record_last_created_token` doc. Both halves are now labelled with their tip and their population. The raw totals above are POSITIVE CONTROLS, not invariants, and they churn in a specific way worth naming: the query matches its own documentation, so writing prose about this ledger moves the number. The previous revision's "36" was already stale when it shipped -- invalidated by the three comment lines the same commit added. What does not churn is the classification, and that half is now enforced executably rather than in prose (see the third census anchor below). The slot has a SECOND publish destination, and the first version of this guard was defeated one line below itself by it. `PendingCopyTokenResolution::created_ids` is the copy-batch buffer, and `token_copy::drain_copy_token_resolution` ends with `state.last_created_token_ids = pending.created_ids;` -- an ASSIGNMENT, not an append. So two sites that guarded the ledger write and then pushed the same id into the buffer as a separate statement republished the withheld id AND overwrote the guarded ledger with the unguarded list. Same discipline: `rg -n --pcre2 -U "pending\s*\.\s*created_ids\s*(\.\s*M\s*\(|=[^=])" crates/engine/src` returns, RUN AT THIS COMMIT, 11 match events, of which 1 is prose in a comment this commit added, leaving 10 production writers and 0 inside `#[cfg(test)]`; exactly ONE of the 10 is a single-id publish, the `push` inside `record_last_created_copy_batch_token`. RUN AT `b654513cb` the same query returns 11 with no comment among them, i.e. 11 production writers, of which TWO published a single just-created id (`counters.rs`'s `FinalizeCopyTokenEntry` arm and `token_copy::apply_remaining_token_modifications_after_counter_pause`). The two tips reading 11 is a COINCIDENCE, not stability -- two single-id pushes were removed while one authority push and one doc comment were added -- and an earlier revision of this message quoted the BASE number as though it described this tip. Those two sites now make ONE call, `token::record_last_created_copy_batch_token`, which performs both writes under ONE evaluation of the predicate. After that, the only CODE occurrence of `pending.created_ids.push(` in `crates/engine/src` is the one inside that authority; the query's second hit is this commit's own doc comment quoting the defect. The `-U` is load-bearing, not decoration: `token_copy.rs:321-323` and `:327-329` split `pending` / `.created_ids` / `.extend(...)` across three lines, so the line-oriented form of the same query reports 9 match events at BOTH tips -- deriving this population from `grep 'last_created_token_ids.push('` is exactly how the earlier sweeps missed sites. CLASS BOUNDARY, named because four prior sweeps each claimed more than they measured -- and then the first revision of THIS paragraph did it a fifth time. CLOSED: "a single just-created object id reaching `last_created_token_ids` or `PendingCopyTokenResolution::created_ids` without the existence predicate", measured by the two queries above over `crates/engine/src`. Its `.push` / `.insert` half is pinned executably by the third census anchor below; the half no text scanner can classify -- `.extend(std::iter::once(id))` and its three siblings, whose ARGUMENT decides whether they publish one id -- is pinned as a whole population by that anchor's conjunct 4 rather than argued away. NOT closed, and unchanged from `b654513cb`: this tip's 15 bulk republishes and 4 clears. An earlier revision of this paragraph said each of the 15 assigns "a clone of ledger 3, a `CopyTokenApplyStatus` built one line after `.expect("token just created")`, or the buffer itself". Measured FALSE: `token.rs:892`, `token.rs:1023` and `token_copy.rs:617` assign a batch seeded from an `initial_created_ids` parameter and extended IN THE LOOP with ids minted in the same call (`token.rs:976`), which is none of those three. What the paragraph needed was a reachability claim, and this is it: each of the 15 assigns a clone of ledger 3, a `CopyTokenApplyStatus`, the buffer itself, OR a batch accumulated inside one uninterrupted call -- and on that fourth path the id cannot have died between mint and publish. The counter-replacement pause `return false`s at `token.rs:941`, BEFORE `created_ids.push(obj_id)` at `:976`; the only other production code between them is `effects::attach`, which performs no object removal (`rg -n --pcre2 -U 'objects\s*\.\s*(remove|retain)\s*\(' crates/engine/src/game/effects/attach.rs` exits 1 with no output, against 1 match for the identical query over `game/zones.rs`; and `attach.rs`'s own five `check_state_based_actions` calls all sit past ITS `#[cfg(test)]` at line 1440 -- naming `attach.rs` again because the preceding clause mentions `zones.rs` and "that file" read as the wrong referent); and no state-based-action check or zone move occurs anywhere in the loop body (`awk 'NR>=774 && NR<=1042'` over `token.rs` for `check_state_based_actions|change_zone|objects\.remove|objects\.retain` prints nothing). Also NOT closed: an id that existed when published and died afterwards, which stays in ledger 3 until the per-turn `.clear()`. That one is pre-existing behaviour and a different question from this fix. `record_last_created_copy_batch_token` is deliberately NOT folded into `record_last_created_token`, and the distinction is behavioural rather than stylistic: that function's other three callers (`counters.rs`'s `FinalizeTokenEntry` and `EmitCommittedCopyTokenEntry` arms, and `finalize_committed_liminal_token_entry_from_action`) do not mirror into the batch buffer, and mirroring there would put a plain or already-batched token into a copy batch's `created_ids`, which the drain then assigns wholesale onto ledger 3 -- silently widening what "the tokens created this way" names. The mirror of that argument is why the predicate stays out of `restrictions::record_token_created`, whose production call sites are a strict SUPERSET of the anaphora slot's, so folding the write in would widen `LastCreated` to `incubate.rs`, `gift_delivery.rs` and the predefined-token arms, which do not claim it. The 3-tuple coherence test fails at the pre-fix tip with `left: (0, 0, 1)` against `right: (0, 0, 0)`, and it was NOT SUFFICIENT. Every fixture in `counters.rs` builds `GameState::new_two_player(42)`, whose resolution stack carries no `ResolutionFrame::CopyToken`, so `state.active_copy_token_mut()` returns `None` and the copy-batch branch is never entered in ANY of the five arms. The test was sensitive to the lines that changed without reaching the branch where the guard could be defeated; sensitivity is not coverage. The new `a_vanished_token_never_reaches_the_anaphora_slot_through_the_copy_batch_buffer` closes that: it pushes a real `CopyToken` frame seeded with an earlier batch's SURVIVOR id, dispatches both routes through `apply_pending_counter_post_action`, and measures its 3-tuple AFTER `drain_pending_copy_token_resolution` performs the wholesale assignment -- the state a `TargetFilter::LastCreated` reference actually reads. Reachability is DEMONSTRATED, not argued: deleting the `if let Some(pending)` body fails the PRESENT row with `left: (0, 0, 1)` against `right: (1, 1, 1)`. Two-sided on one named assertion, the gone row's `(0, 0, 1)`: MUTANT-DROP (delete the guard's early return -- the exact shape that shipped) gives `(1, 1, 1)`, MUTANT-TRIVIALIZE (keep every branch, force `contains_key` to `true`) gives `(1, 1, 1)`, and both leave the PRESENT row passing. The survivor column stays 1 under every mutant, which is what makes the two zeros a measurement of the guard rather than of a drain that never ran. This replaces per-call-site guarding, which was the wrong instrument: three successive sweeps of the eight call sites each missed a route, and each shipped a claim that the sweep was complete. Guarding inside the shared emitter makes the class unrepresentable instead of enumerated, and deletes the caller-side guards in the process. An earlier revision of this message claimed a specific net production line count here; that figure was a delta against an unpublished intermediate candidate and is not reproducible from this commit, so it is dropped rather than restated. The figure for this commit's own range is whatever `git diff --numstat b654513cb..HEAD -- 'crates/engine/src/*'` reports at this commit. The census enforces both halves of the pair AND the anaphora slot behind them, so none of the three is left to enumeration: it pins production `ZoneChanged { from: None }` constructions to the record+emit authority, production `GameEvent::TokenCreated` constructions to this emitter, and production single-id publishes into either anaphora container to the two `record_last_created_*` functions -- each with a function-scope conjunct so a clone written inside the owning file that also deleted the original's hit cannot pass. Without the guard the emit is not merely a ledger disagreement. `match_token_created` reads the controller inside `if let Some(token_controller) = state.objects.get(..)`, and `valid_card_matches` short-circuits `None => true` without reading state, so a vanished token never has the CR 111.2 controller filter applied and the matcher returns true -- a trigger fire for a NON-MATCHING controller. Measured: with `valid_card = None` the gone path matches where the present path does not; with `valid_card = Typed{Creature}` it does not, which is the negative control. The class-wide fix is to resolve the controller through last-known information the way `valid_card_matches_with_lki` already does for the card filter; that touches a shared matcher and is recorded as follow-up rather than done here. Because the predicate sits in the shared emitter, it covers all five pause-deferred routes -- `counters.rs`'s `FinalizeTokenEntry`, `FinalizeCopyTokenEntry`, `ApplyCopyTokenModificationsAndFinalize` and both `FinalizeCommittedLiminalTokenEntry` arms -- including `token.rs`'s `flush_pending_token_battlefield_entry`, which earlier revisions left unguarded and merely disclosed. It also covers the non-deferred creation paths, a wider blast radius than a per-caller guard: that is deliberate, and the whole suite plus an equivalence probe asserting `record.is_some() == state.objects.contains_key(..)` at every invocation found nothing reachable that depends on the old behaviour. That probe was executed at the round-6 candidate, where the suite was 22979 tests; at this tip the battery is 23049 passed and 15 ignored (18496 + 12 + 9 + 4532 + 0). The number is stated against the candidate it was measured at rather than presented as a fact about this commit -- the difference is six tests added by later review rounds, plus the 64 upstream added between `4b34e5465` and this commit's base `b654513cb`, which the rebase brought in. The two `.expect(...)` call sites still panic identically -- the panic is on the returned `Option`, which is unchanged. `phase-engine` contains no production `catch_unwind`: `rg -c 'std::panic::catch_unwind\(' crates/engine/src` reports 2, both `#[cfg(test)]`-scoped (`game/layers.rs:20642` under the `cfg(test)` at :8573, and `parser/oracle_static/tests.rs:1230`). The catchers that DO exist are consumer-side in `phase-ai`, and there are FIVE, not the two an earlier revision listed: `rg -n 'std::panic::catch_unwind\(' crates/phase-ai/src/` gives `bin/ai_tune.rs:769`, `bin/ai_commander.rs:832` and `:833`, and `duel_suite/run.rs:584` and `:604`. None of the five can observe the difference: `events` is a call-local `Vec` dropped during the unwind, so whether a `TokenCreated` was pushed before the panic is unobservable from any catcher. The conclusion is unaffected; only the enumeration was incomplete. STILL OPEN, recorded rather than fixed: `match_token_created` resolves the token's controller by reading the live object, so for a gone object it skips the CR 111.2 controller filter entirely. Suppressing the emit makes event and ledger agree; the deeper fix is to emit with a last-known-information controller the way `valid_card_matches_with_lki` already does for the card filter. That touches a shared matcher, so it is follow-up -- and the emitter is now its single adoption point. No CR settles whether a token that never successfully entered should fire a creation trigger -- CR 704.3 checks state-based actions only when a player would get priority, so nothing can remove the token between its creation and the CR 603.6a check, and the arm is a defensive engine artifact of deferring the emit past a replacement pause. The requirement on it is internal agreement between event and ledger, not a rules verdict. The entry-ledger annotations cite CR 608.2i, not CR 403.3. CR 403.3 is definitional ("Permanents exist only on the battlefield"); it does not describe an entry-time characteristics snapshot kept for later look-back queries. All seven production consumers of `battlefield_entries_this_turn` are conditions, quantities, or a static analysis probe evaluated during resolution -- none is a trigger condition -- so CR 603.10's look-back-for-triggering rule does not apply either. CR 608.2i ("Some effects look back in time and require information about previous game states") is what the pre-existing `battlefield_entry_record_for` already cites. The retag covers every site in this commit's touch-set, including the one survivor inside it (`zones::retag_battlefield_entry_snapshots`); the remaining pre-existing CR 403.3 entry-ledger citations are in files this commit does not modify -- `restrictions.rs` and `types/game_state.rs`, which carry 8 and 5 CR 403.3 citations respectively, both counts unchanged from `b654513cb` -- and are recorded as follow-up. Those per-file totals include non-entry-ledger uses (definitional sites and test assertions): an earlier revision of this message enumerated five coordinates as though they were the exhaustive entry-ledger subset, and they are not, so the follow-up triages the 13 rather than assuming each needs the retag. Separately, `effects/token.rs`'s two CR 111.10 citations for the token CREATION EVENT are retagged to CR 111.1 ("Some effects put tokens onto the battlefield"). That retag is deliberately narrow: CR 111.10 is the predefined-token characteristics catalog, which the file's ~58 other CR 111.10 citations use correctly, and a blanket retag would corrupt them. `game/restrictions.rs` is deliberately untouched: it is the ledger authority (one `GameEvent` emission in 5166 lines, never a `ZoneChanged`), so the fused emitter belongs beside the existing emitter in `game/zones.rs`. Adds a structural census test asserting every production no-origin-zone `ZoneChanged` construction in `crates/engine/src` lives in the single authority, so a SEVENTH clone trips CI rather than silently shipping a placeholder index. It is a source-text tripwire against an accidental clone, not a proof that one cannot be written, and its residual ceilings are enumerated in its own module header. It scans each literal's own brace extent and classifies the `from` FIELD -- both the spelling `from: None` and field-init shorthand -- because an earlier form keyed on the substring `from: None` inside a fixed six-line window and BOTH halves were evadable by code that compiles (`let from = None;` plus `from,`; and a multi-line `record:` initializer written first, which pushes `from: None` past the window). Review found THREE further evasions of that brace scan, and unlike the first two they are FAIL-OPEN: a `}` inside a `//` comment, inside a string literal, or inside a NESTED block comment closed the literal early and TRUNCATED the captured body, so every field after it went unseen and a real construction scored zero. The first two are closed by skipping comments and string / raw-string / char literals. The third survived that very rewrite, because block-comment state was a BOOL: Rust block comments nest, so `/* outer /* inner */ } */` left comment state at the inner `*/` and the `}` was counted -- and the rewrite's residual list, copied from its predecessor rather than read off the scanner, did not name it. It is closed with a depth counter. A FOURTH fail-open truncation, found in review round 12, closes here too: RAW BYTE and RAW C strings (`br#"..."#`, `cr#"..."#`). `b` and `c` are alphanumeric, so the guard that stops an identifier's trailing `r` opening a raw string rejected these, and the `"` after it fell through to the `#`-blind escaped-string scan, which stops at the first embedded `"` and lets the following `}` close the literal. Measured against a verbatim extraction of the shipped scanner, before and after the one-line `literal_prefix_start` fix: `br#"a " } b"#` scores `(0, 0)` -> `(1, 1)` and `cr#"..."#` the same, while the byte-identical control `r#"a " } b"#` scores `(1, 1)` under BOTH scans. All three forms lex as valid Rust (`rustc --edition 2021 --crate-type=lib`, rc 0). Latent, not live: `br` / `cr` occur ZERO times in `crates/engine/src` (`rg --pcre2 -U --json '(? 0 match events, with a synthetic positive control (the `printf` that builds it is in the doc) that the same query scores 1 on. The verdict is unchanged -- there is no live instance -- but the evidence now supports it. (2) "No anchor literal in `engine/src` contains a string at all" was false. Measured over the exact population this scanner walks: of 326 `ZoneChanged` and 38 `TokenCreated` anchor lines, 20 and 10 respectively DO carry a string inside the literal's extent, and ZERO carry one spanning a line break -- which is the truncating form the residual is actually about. The old claim was false in the direction that stops a reader looking for the multi-line case. `FN_PREFIX_ALLOW_SET` also gains `pub(in crate::game) `: the census's own abort told the reader to extend the allow-set with exactly the prefix another arm hard-coded as its "unrecognised" fixture, so obeying the error turned one red test into another. The prefix counts that back this up are re-measured AT THIS COMMIT'S TREE and shipped with the command that produces them (a `find ... -exec awk` one- liner printed in `FN_PREFIX_ALLOW_SET`'s doc, implementing exactly the rule `top_level_fn_headers` applies): 14021 bare, 1813 `pub(crate) `, 1492 `pub `, 734 `pub(super) `, 24 `pub(in crate::game) `, and exactly FIVE distinct prefixes. An earlier revision shipped the BASE-tip `1808 pub(crate) ` beside the HEAD-tip `14008 bare`, a pair that existed at no commit; the four absolute counts are now labelled as positive controls that churn on any top-level `fn` addition, while the two facts the arms depend on -- five distinct prefixes, and `const ` not among them (`rg -n --pcre2 '^const\s+fn\s' crates/engine/src` exits 1) -- are re-derivable from the same commands. A THIRD ANCHOR closes the enforcement gap this commit was otherwise shipping. The two anchors above pin the two halves of the entry EVENT; the defect that actually shipped in review rounds 9 and 10 was neither, and it had no tripwire at all. It was a single just-created id published into the anaphora slot without the existence predicate -- first into `last_created_token_ids` directly, then, once that was guarded, into `pending.created_ids` one line BELOW the guard, which the drain's wholesale assign republished onto ledger 3. Four enumeration sweeps each declared that class closed, and its closure then rested on a query written in a doc comment -- i.e. on the ENUMERATION this commit argues against, enforcing its own newest authority. `every_single_id_anaphora_publish_lives_in_an_authority` is that query made executable, and the word "executable" is doing real work, so here is exactly how much of the query it covers. The query's mutator set is twelve verbs. Two of them -- `.push(`, `.insert(` -- introduce exactly one element whatever their argument, and CONJUNCTS 1-3 pin those to the production multiset `{effects/token.rs: 2}`, `(2, 0)` bidirectionally, with both hits' enclosing functions resolved to `record_last_created_token` and `record_last_created_copy_batch_token`. Six -- `.clear(`, `.retain(`, `.truncate(`, `.remove(`, `.drain(`, `.pop(` -- cannot introduce an element at all, which is a property of the method rather than of this tree. The remaining four -- `.extend(`, `.append(`, `.resize(`, `.splice(` -- introduce one id or many depending on an ARGUMENT no text scanner can read: `.extend(std::iter::once(id))` publishes exactly one id and is byte-indistinguishable in shape from `.extend(other_vec)`. CONJUNCT 4 therefore does not classify them; it pins their entire production population to `(5, 0)` and the multiset `{counters.rs: 2, token.rs: 1, token_copy.rs: 2}`, so a sixth in ANY OF THE EIGHT LISTED VERBS cannot arrive without a human reading its argument. That scope is stated because an earlier revision claimed it without one: the set then held four verbs, and `extend_from_slice`, `clone_from`, `resize_with` and `extend_from_within` each compile, each publish exactly one id, and each scored `(0, 0)` under it. They are listed now; any `std` mutator still unlisted is a named fail-OPEN in the residual list, not a denied one. That converts an unbounded fail-OPEN into a bounded fail-CLOSED, which is the discipline this file already applies to an unreadable `/* ... */` tail and to an unrecognised `fn` prefix. Conjunct 4 is a SEPARATE pin rather than four more verbs in conjunct 1, and the reason is measured: folding them in takes the pin to `(7, 0)`, puts `counters.rs` and `token_copy.rs` -- whose ABSENCE is conjunct 1's entire claim -- back into the multiset, and adds a non-authority function (`apply_create_token_after_replacement_with_created_ids`) to conjunct 2's list. Two pins state the two facts; one pin would state neither. What is NOT covered, and is named in the file's residual list with its own measured query and positive control: an indexed assignment (`...[0] = id`), which shares the `[` tail with 26 indexed READS (25 of them `#[cfg(test)]` fixtures, so counting `[` would move the pin to `(3, 25)`, the production multiset to `{database/encore_tests.rs: 1, effects/token.rs: 2}`, and the files touched from one to seven, churning on every new test that reads the ledger), and a publish through an alias or the UFCS `Vec::push(&mut state.last_created_token_ids, id)` form. It shares `cfg_test_scoped_lines`, the file walker and the `top_level_fn_headers` / `enclosing_fn` resolver with the other two anchors, but deliberately NOT `literal_body`: it keys on a method CALL, not a struct-literal body, so it has no brace scan and inherits none of the four fail-open truncations above. It is not line-oriented either -- `call_tail` reads the next code token across line breaks, because `token_copy.rs:321-323` writes `pending` / `.created_ids` / `.extend(...)` on three lines and a line-oriented rule is exactly what under-counted this surface before. Its one comment rule, `code_span`, is subtractive-only and each half is guarded so it cannot delete code: a leading `/* ... */` is dropped only up to the first `*/` (so `engine.rs`'s `/*tapped=*/` argument-label shape keeps its call), and a trailing `//` only when no `"` precedes it (so a URL cannot hide a publish). Both guards have their own control arm. The anti-vacuity set is eight arms: the round-9 shape at both scopes; the round-10 multi-line buffer shape; a negative control over every non-publish tail form that occurs in the crate, whose message now says only what is true of each form (a bulk assign, `.clear`, `.clone`, `.len`, indexing and `.contains` cannot introduce an id; `.extend` CAN, and is uncounted there solely because its argument is unreadable -- an earlier revision asserted the universal "none of them introduces an id", which was false for exactly the verb the anchor was built for); a full-line prose negative control; `.insert(` as the other single-element write; a fail-CLOSED arm proving an unreadable `/* ... */` tail is COUNTED rather than skipped; a partition arm that asserts `.extend(std::iter::once(id))` scores `(0, 0)` under the single-id classifier and `(1, 1)` under conjunct 4's, and `.push(id)` the reverse, so the two populations can never double-count or absorb each other's churn; and a comment-position arm covering the trailing-`//` and full-line-`/*` prose shapes plus the two controls that prove `code_span` cannot eat code. The anchor was verified to go RED on the defect it exists to catch, by mutating a scratch copy of `crates/engine/src` and re-running the extracted classifier: reintroducing round 10's unguarded `pending.created_ids.push(id)` beside the guard at `counters.rs` fails the pin, the same at `token_copy.rs` fails it, a bare `state.last_created_token_ids.push(id)` (the round 8/9 shape) fails it, and a clone written INSIDE `token.rs` that also deletes one authority's own push -- which keeps the per-file multiset at 2 and passes conjunct 1 -- is caught by the function-scope conjunct instead. Four mutants, four reds, each on the conjunct it is meant to exercise. Test provenance: the integration suite is salvaged from a halted predecessor implementation. Its assertions carry inline REVERT-PROBE anchors naming, per assertion, the mutation that should break it. Seven of the twenty-five anchors were executed at this tip and carry verbatim failure points; the rest are stated but not run here. The module header now defines that convention explicitly -- RUN is an instruction to the reader, MEASURED means executed with a transcribed failure point -- and names both sets by test, because the gap between "stated" and "executed" is what let two anchors name failure points their recipes never reached. It also records the recipe-scope failure mode behind that: an authority-wide revert degrades the fixture's priming producer too and can trip an upstream guard before the named assertion, which a site-isolated revert does not. Every assertion added or altered in each review round's own delta was proved two-sided per-assertion, at the tip that round produced: MUTANT-DROP (delete the fix) and MUTANT-TRIVIALIZE (keep the shape, make the recorded index a meaningless constant) must each fail that specific assertion, not merely the suite. That is a claim about the assertions each round touched, not about all twenty-five at once. The distinctness check between a realized copy entry and a same-turn token batch reads both indices off their own emitted events, because the dedup guard keys on the event-borne index; an earlier form compared a ledger POSITION against event-borne STORED indices and so passed under both of its own mutants. `ledger_index` stays positional on purpose -- its other three callers compare it against the same entry's stored index, which is the invariant "the recorder reported the slot it assigned", and unifying it on the stored field would turn those into tautologies. Assisted-by: ClaudeCode:claude-opus-5 --- crates/engine/src/game/effects/conjure.rs | 32 +- crates/engine/src/game/effects/counters.rs | 569 ++++- .../engine/src/game/effects/gift_delivery.rs | 30 +- crates/engine/src/game/effects/incubate.rs | 32 +- crates/engine/src/game/effects/token.rs | 321 ++- crates/engine/src/game/effects/token_copy.rs | 71 +- crates/engine/src/game/zones.rs | 76 +- .../battlefield_entry_authority_census.rs | 1889 +++++++++++++++++ .../loop_shortcut_offer_writer_census.rs | 4 +- crates/engine/tests/integration/main.rs | 1 + .../integration/token_zone_change_index.rs | 1452 ++++++++++++- 11 files changed, 4193 insertions(+), 284 deletions(-) create mode 100644 crates/engine/tests/integration/battlefield_entry_authority_census.rs diff --git a/crates/engine/src/game/effects/conjure.rs b/crates/engine/src/game/effects/conjure.rs index 082b803211..7ff40baafc 100644 --- a/crates/engine/src/game/effects/conjure.rs +++ b/crates/engine/src/game/effects/conjure.rs @@ -186,9 +186,7 @@ pub fn resolve( } } - // Record battlefield entry for restriction tracking. if destination == Zone::Battlefield { - crate::game::restrictions::record_battlefield_entry(state, obj_id); // Battlefield entry: incremental re-derive candidate for this // conjured object (escalates to Full if it sources effects/etc.). crate::game::layers::mark_layers_entered(state, obj_id); @@ -202,20 +200,22 @@ pub fn resolve( // (e.g. Verdant Dread's "another Verdant Dread enters" manifest-dread // trigger, Soul Warden, Panharmonicon). Without this the conjured // permanent enters silently and no ETB ability ever triggers. - let zone_change_record = state - .objects - .get(&obj_id) - .expect("conjured object was just created") - .snapshot_for_zone_change(obj_id, None, Zone::Battlefield); - state - .zone_changes_this_turn - .push_back(zone_change_record.clone()); - events.push(GameEvent::ZoneChanged { - object_id: obj_id, - from: None, - to: Zone::Battlefield, - record: Box::new(zone_change_record), - }); + // + // Conjuring is an Alchemy/Arena digital-only mechanic with NO + // Comprehensive Rules entry — the string "conjure" does not occur in the + // CR. The rules cited here are the ones the operation borrows: CR 400.7 + // (the zone change), CR 608.2i (the battlefield-entry bookkeeping), + // CR 603.2c + CR 603.6a (why the index is load-bearing for batched ETB + // triggers). + // + // CR 400.7 + CR 608.2i + CR 603.2c: route the record and the emit through + // the single `from: None → Battlefield` authority so the emitted record + // carries this turn's real zone-change index instead of the `0` + // placeholder, and so the CR 608.2i battlefield-entry row is written + // exactly once (the authority calls `record_battlefield_entry` itself — + // a co-located second call here would double-count it). + crate::game::zones::record_and_emit_entry_from_no_zone(state, obj_id, events) + .expect("conjured object was just created"); } events.push(GameEvent::ObjectConjured { diff --git a/crates/engine/src/game/effects/counters.rs b/crates/engine/src/game/effects/counters.rs index e01a3e48ed..abbc025b74 100644 --- a/crates/engine/src/game/effects/counters.rs +++ b/crates/engine/src/game/effects/counters.rs @@ -515,30 +515,19 @@ fn apply_pending_counter_post_action( // replacement-processed counters finish. super::token::inject_predefined_token_abilities(state, object_id); crate::game::layers::mark_layers_entered(state, object_id); - crate::game::restrictions::record_battlefield_entry(state, object_id); crate::game::restrictions::record_token_created(state, object_id); // CR 603.6a: finalize the deferred ZoneChanged here, once the // token's counters have actually settled, so ETB trigger // observers (Altar of the Brood, Soul Warden, etc.) see the // Incubator's final counter count rather than firing early on a // pre-replacement-choice snapshot (issue #4238). - if let Some(zone_change_record) = state.objects.get(&object_id).map(|obj| { - obj.snapshot_for_zone_change( - object_id, - None, - crate::types::zones::Zone::Battlefield, - ) - }) { - state - .zone_changes_this_turn - .push_back(zone_change_record.clone()); - events.push(GameEvent::ZoneChanged { - object_id, - from: None, - to: crate::types::zones::Zone::Battlefield, - record: Box::new(zone_change_record), - }); - } + // + // CR 400.7 + CR 608.2i + CR 603.2c: route the record and the emit through the single + // `from: None → Battlefield` authority so the emitted record carries this turn's real + // zone-change index instead of the `0` placeholder. The authority calls + // `record_battlefield_entry` itself, so the co-located call that used to sit above is + // deleted — keeping it would double-count `battlefield_entries_this_turn`. + crate::game::zones::record_and_emit_entry_from_no_zone(state, object_id, events); true } PendingCounterPostAction::FinalizeTokenEntry { @@ -555,7 +544,6 @@ fn apply_pending_counter_post_action( // delayed sacrifice trigger. super::token::inject_resolved_token_abilities(state, object_id); crate::game::layers::mark_layers_entered(state, object_id); - crate::game::restrictions::record_battlefield_entry(state, object_id); crate::game::restrictions::record_token_created(state, object_id); if let Some(host) = attach_to { match host { @@ -567,7 +555,23 @@ fn apply_pending_counter_post_action( } } } - push_token_entry_events(state, events, object_id, name, source_id); + // CR 400.7 + CR 608.2i + CR 603.2c: route the record and the entry pair through the + // single `from: None → Battlefield` authority so the emitted `ZoneChanged` carries + // this turn's real zone-change index. The authority calls `record_battlefield_entry` + // itself, so the co-located call that used to precede the attachment block is + // deleted; that also moves the CR 608.2i snapshot point to AFTER `attach_to`'s + // synchronous layer flush, so an attached token's entry row records its post-flush + // characteristics (sanctioned by `battlefield_entry_record_for`'s own doc). + // + // OBJECT-GONE: if the token is no longer in `state.objects` when its parked counters + // settle, this route reports NOTHING — no CR 400.7 row, no `ZoneChanged`, and no + // `TokenCreated`. No guard here: `push_committed_token_entry_events` withholds + // `TokenCreated` on the authority's own `None` verdict, which is the same predicate + // that keeps `restrictions::record_token_created` above from writing a row. See that + // function's doc for the wrong-trigger-fire measurement this prevents. + super::token::push_committed_token_entry_events( + state, object_id, name, source_id, events, + ); if matches!(sacrifice_at, Some(Duration::UntilEndOfCombat)) { let sacrifice_token = DelayedTrigger { condition: DelayedTriggerCondition::AtNextPhase { @@ -590,7 +594,7 @@ fn apply_pending_counter_post_action( }; crate::game::triggers::install_delayed_trigger(state, sacrifice_token, events); } - state.last_created_token_ids.push(object_id); + super::token::record_last_created_token(state, object_id); true } PendingCounterPostAction::ContinueTokenCreation { @@ -634,13 +638,28 @@ fn apply_pending_counter_post_action( } super::token::inject_predefined_token_abilities(state, object_id); crate::game::layers::mark_layers_entered(state, object_id); - crate::game::restrictions::record_battlefield_entry(state, object_id); crate::game::restrictions::record_token_created(state, object_id); - push_token_entry_events(state, events, object_id, name, source_id); - state.last_created_token_ids.push(object_id); - if let Some(pending) = state.active_copy_token_mut() { - pending.created_ids.push(object_id); - } + // CR 400.7 + CR 608.2i + CR 603.2c: route the record and the entry pair through the + // single `from: None → Battlefield` authority so the emitted `ZoneChanged` carries + // this turn's real zone-change index. The authority calls `record_battlefield_entry` + // itself, so the co-located call that used to live here is deleted — keeping it would + // double-count `battlefield_entries_this_turn` for every `FinalizeCopyTokenEntry` + // token. + // + // OBJECT-GONE: same contract as the `FinalizeTokenEntry` arm above — a token that is + // no longer in `state.objects` when its parked counters settle reports nothing, + // because `push_committed_token_entry_events` gates `TokenCreated` on the authority's + // `None` verdict, so it never disagrees with the existence-guarded + // `record_token_created` ledger write immediately above it. + super::token::push_committed_token_entry_events( + state, object_id, name, source_id, events, + ); + // The anaphora slot has TWO destinations on this route — ledger 3 and the in-flight + // copy batch's `created_ids`, which `token_copy.rs`'s drain assigns WHOLESALE back onto + // ledger 3 — so one guarded call owns both. Guarding the ledger write and pushing the + // buffer as a separate statement republished the withheld id and overwrote the guarded + // list with it. + super::token::record_last_created_copy_batch_token(state, object_id); true } PendingCounterPostAction::ContinueCopyTokenCreation { @@ -733,8 +752,14 @@ fn apply_pending_counter_post_action( // (structurally idempotent, `Option::take_if`), which is not an error. let _ = super::token::flush_pending_token_battlefield_entry(state, object_id, events); if !state.last_created_token_ids.contains(&object_id) { - state.last_created_token_ids.push(object_id); + super::token::record_last_created_token(state, object_id); } + // DELIBERATELY NOT `record_last_created_copy_batch_token`: this arm RE-SYNCS the batch + // buffer to the whole guarded ledger rather than appending one id, and the two are not + // interchangeable — the buffer accumulates across batches while the ledger is reset per + // batch. Copying the ledger cannot publish an id the ledger's own guard withheld, so + // this shape needs no second predicate; it needs the source to stay the GUARDED ledger, + // which is what `state.last_created_token_ids` is after the call above. let created_ids = state.last_created_token_ids.clone(); if let Some(pending) = state.active_copy_token_mut() { pending.created_ids = created_ids; @@ -824,31 +849,6 @@ fn apply_pending_counter_post_action( } } -fn push_token_entry_events( - state: &GameState, - events: &mut Vec, - object_id: ObjectId, - name: String, - source_id: ObjectId, -) { - let Some(obj) = state.objects.get(&object_id) else { - return; - }; - let zone_change_record = - obj.snapshot_for_zone_change(object_id, None, crate::types::zones::Zone::Battlefield); - events.push(GameEvent::ZoneChanged { - object_id, - from: None, - to: crate::types::zones::Zone::Battlefield, - record: Box::new(zone_change_record), - }); - events.push(GameEvent::TokenCreated { - object_id, - name, - source_id, - }); -} - /// CR 122.1 + CR 122.6: Apply an already-accepted counter addition and record /// the actor/recipient snapshot for "counters you've put this turn" quantities. pub(crate) fn apply_counter_addition( @@ -6223,4 +6223,467 @@ mod tests { "destination must receive the counter after move" ); } + + /// COHERENCE INVARIANT for EVERY route that defers a token-entry emit past a pause: **none + /// reports a creation the others do not.** The tuple is `(TokenCreated events, + /// created_tokens_this_turn rows, last_created_token_ids entries)` — all THREE ledgers a token + /// creation writes, not the two that share a guard. + /// + /// The third entry is why this assertion is a 3-tuple. Gating the emit on the authority's + /// `record.is_some()` verdict made the event agree with `created_tokens_this_turn` and + /// `players_who_created_token_this_turn`, but `last_created_token_ids` — the + /// `TargetFilter::LastCreated` anaphora slot — was written unguarded on every one of these + /// routes, so the gone path read `(0, 0, 1)`: the change FLIPPED which ledger the event + /// disagreed with instead of removing the disagreement, and a 2-tuple is exactly the projection + /// under which that is invisible. `token::record_last_created_token` now carries the same + /// existence predicate; MEASURED at the pre-fix tip, this test fails on its first arm with + /// `left: (0, 0, 1)` against `right: (0, 0, 0)`. + /// + /// `restrictions::record_token_created` is existence-guarded, so if the emit is not gated on + /// the same predicate a vanished token puts a live trigger event + /// (`trigger_matchers::match_token_created`, keyed in `trigger_index`) on the wire that no + /// ledger row backs — and that matcher then skips its CR 111.2 controller filter entirely, so + /// it fires for a controller it should have rejected (measurement in + /// `token::push_committed_token_entry_events`'s doc). This restores the pre-change behaviour + /// of the deleted `counters::push_token_entry_events`, whose `let Some(obj) = … else { return; + /// }` head emitted nothing on the gone path. + /// + /// COVERAGE IS THE WHOLE CLASS, not an enumeration of files. The single predicate lives inside + /// `token::push_committed_token_entry_events`, the ONLY production emit of + /// `GameEvent::TokenCreated`, so every one of its EIGHT callers inherits it. The five arms + /// below are the five callers whose emit is separated from the object's creation by a pause — + /// i.e. every caller on which the object can genuinely be gone. The remaining three emit + /// inside the same call that created the object — + /// `token::apply_create_token_after_replacement_with_created_ids`, + /// `gift_delivery::create_gift_token`, and + /// `token_copy::apply_copy_token_after_replacement_with_created_ids` — and the latter two + /// additionally `.expect(…)` the record so a vanished object panics rather than disagreeing + /// silently. + /// + /// NO CR settles whether a token that never successfully entered should fire a creation + /// trigger, because the situation is unreachable in rules terms: CR 704.3 checks state-based + /// actions only "whenever a player would get priority", so nothing can remove the token between + /// its creation and the CR 603.6a enters-the-battlefield check. The gone arm is a defensive + /// engine artifact of deferring the emit past a replacement pause, and the engineering + /// requirement on it is internal agreement, not a rules verdict. + /// + /// TWO-SIDED, each mutant failing the SAME gone-path assertion on EVERY arm: + /// * MUTANT-DROP — delete the `if record.is_some()` in `push_committed_token_entry_events` ⇒ + /// `token_created` reads 1 while both turn ledgers read 0. + /// * MUTANT-TRIVIALIZE — keep the branch's shape, make it `if true` ⇒ same flip, same + /// assertion. + /// * MUTANT-DROP-3 — delete the `contains_key` in `token::record_last_created_token` ⇒ the + /// gone path reads `(0, 0, 1)`, which is the pre-fix behaviour and the third entry's own + /// revert probe. + /// + /// All three leave the `(1, 1, 1)` positive control passing, so no mutant is caught merely by a + /// fixture that never ran the arm. + #[test] + fn a_vanished_counter_paused_token_reports_neither_creation_event_nor_ledger_row() { + /// The five routes whose token-entry emit is deferred past a pause. All are driven through + /// their production entry point: every arm is a `PendingCounterPostAction` variant + /// dispatched by `apply_pending_counter_post_action`, and the last one parks its entry + /// through that same dispatcher and is then realized by + /// `token::flush_pending_token_battlefield_entry`, exactly as + /// `token::realize_settled_token_battlefield_entry` does from `apply_action`. + enum Route { + FinalizeTokenEntry, + FinalizeCopyTokenEntry, + ApplyCopyTokenModificationsAndFinalize, + LiminalEntryEmit, + LiminalEntryFlush, + } + + fn run(route: &Route, present: bool) -> (usize, usize, usize) { + let mut state = GameState::new_two_player(42); + let source_id = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Token Source".to_string(), + Zone::Battlefield, + ); + let object_id = create_object( + &mut state, + CardId(0), + PlayerId(0), + "Saproling".to_string(), + Zone::Battlefield, + ); + let liminal_entry = + |entry_events| PendingCounterPostAction::FinalizeCommittedLiminalTokenEntry { + object_id, + name: "Saproling".to_string(), + source_id, + controller: PlayerId(0), + enters_attacking: false, + attach_to: None, + sacrifice_at: None, + created_ids: Vec::new(), + ability_injection: + crate::types::game_state::LiminalTokenAbilityInjection::ResolvedToken, + entry_events, + }; + let action = match route { + Route::FinalizeCopyTokenEntry => PendingCounterPostAction::FinalizeCopyTokenEntry { + object_id, + name: "Saproling".to_string(), + enters_attacking: false, + source_id, + controller: PlayerId(0), + }, + Route::FinalizeTokenEntry => PendingCounterPostAction::FinalizeTokenEntry { + object_id, + name: "Saproling".to_string(), + attach_to: None, + sacrifice_at: None, + source_id, + controller: PlayerId(0), + }, + // Empty `remaining_modifications`: `apply_token_modifications` returns `true` on an + // empty slice, so the resume reaches its emit tail — the statement under test — + // without needing an `, except` body. + Route::ApplyCopyTokenModificationsAndFinalize => { + PendingCounterPostAction::ApplyCopyTokenModificationsAndFinalize { + object_id, + name: "Saproling".to_string(), + enters_attacking: false, + source_id, + controller: PlayerId(0), + remaining_modifications: Vec::new(), + } + } + Route::LiminalEntryEmit => { + liminal_entry(crate::types::game_state::TokenEntryEventEmission::Emit) + } + Route::LiminalEntryFlush => { + liminal_entry(crate::types::game_state::TokenEntryEventEmission::Suppress) + } + }; + if !present { + // CR 704.5f is the live way to get here (a 0-toughness copy buried while its + // counter-ordering prompt was open); removing the object models the same end state + // without staging a whole SBA pass. Removed BEFORE the resume, which is where the + // window actually is: the object is committed to the battlefield before the + // counter replacement pauses, so it can only vanish while the prompt is open. + state.objects.remove(&object_id); + state.battlefield.retain(|id| *id != object_id); + } + let mut events = Vec::new(); + apply_pending_counter_post_action(&mut state, action, &mut events); + if matches!(route, Route::LiminalEntryFlush) { + assert!( + state + .pending_token_battlefield_entry + .as_ref() + .is_some_and(|pending| pending.object_id == object_id), + "REACH-GUARD: the Suppress arm must have parked the entry, or the flush below \ + is a no-op and its counts prove nothing" + ); + assert!( + crate::game::effects::token::flush_pending_token_battlefield_entry( + &mut state, + object_id, + &mut events, + ), + "REACH-GUARD: the flush must consume the parked entry" + ); + } + ( + events + .iter() + .filter(|event| matches!(event, GameEvent::TokenCreated { .. })) + .count(), + state.created_tokens_this_turn.len(), + state.last_created_token_ids.len(), + ) + } + + for (route, arm) in [ + (Route::FinalizeTokenEntry, "counters::FinalizeTokenEntry"), + ( + Route::FinalizeCopyTokenEntry, + "counters::FinalizeCopyTokenEntry", + ), + ( + Route::ApplyCopyTokenModificationsAndFinalize, + "counters::ApplyCopyTokenModificationsAndFinalize -> \ + token_copy::apply_remaining_token_modifications_after_counter_pause", + ), + ( + Route::LiminalEntryEmit, + "counters::FinalizeCommittedLiminalTokenEntry (entry_events: Emit)", + ), + ( + Route::LiminalEntryFlush, + "counters::FinalizeCommittedLiminalTokenEntry (entry_events: Suppress) -> \ + token::flush_pending_token_battlefield_entry", + ), + ] { + // POSITIVE CONTROL — the instrument can report non-zero, so the zeros below are a + // measurement and not a fixture that never ran the arm. + assert_eq!( + run(&route, true), + (1, 1, 1), + "{arm}: with the token still on the battlefield the route emits exactly one \ + TokenCreated AND writes exactly one created_tokens_this_turn row AND publishes \ + exactly one last_created_token_ids entry" + ); + + // THE INVARIANT: event and ALL THREE ledgers agree on the gone path too. + assert_eq!( + run(&route, false), + (0, 0, 0), + "{arm}: a vanished token must emit NO TokenCreated and appear in NO token-creation \ + ledger, because the object it names does not exist. Dropping the \ + `if record.is_some()` in `push_committed_token_entry_events` yields (1, 0, 0) — a \ + live creation trigger event backed by an empty ledger. Dropping \ + `token::record_last_created_token`'s existence guard yields (0, 0, 1) — a dead \ + object id published into the `TargetFilter::LastCreated` anaphora slot" + ); + } + } + + /// The SIXTH `last_created_token_ids` writer, and the one the five arms above do not reach: + /// `PendingCounterPostAction::EmitCommittedCopyTokenEntry`. + /// + /// Its shape differs, which is why it gets its own assertion rather than a sixth arm. The + /// CR 400.7 row and the `created_tokens_this_turn` write happen UPSTREAM of the counter pause + /// on this route, so the 3-tuple invariant above does not describe it — on the gone path this + /// variant legitimately leaves an earlier turn-ledger row alone and its `flush` is a no-op. + /// What DOES apply is the third ledger on its own: a `TargetFilter::LastCreated` reference must + /// never name an object that is no longer in `state.objects`. + /// + /// Two-sided on the guard it covers: deleting `contains_key` in + /// `token::record_last_created_token` flips the gone row from 0 to 1 while leaving the positive + /// control at 1, so the assertion discriminates rather than merely running. + #[test] + fn a_vanished_copy_token_is_not_published_to_the_last_created_anaphora_slot() { + fn run(present: bool) -> (bool, usize) { + let mut state = GameState::new_two_player(42); + let object_id = create_object( + &mut state, + CardId(0), + PlayerId(0), + "Copy Token".to_string(), + Zone::Battlefield, + ); + assert!( + state.last_created_token_ids.is_empty(), + "REACH-GUARD: the slot must start empty, or the count below is not attributable to \ + this dispatch" + ); + if !present { + state.objects.remove(&object_id); + state.battlefield.retain(|id| *id != object_id); + } + let mut events = Vec::new(); + let handled = apply_pending_counter_post_action( + &mut state, + PendingCounterPostAction::EmitCommittedCopyTokenEntry { object_id }, + &mut events, + ); + ( + handled, + state + .last_created_token_ids + .iter() + .filter(|id| **id == object_id) + .count(), + ) + } + + // POSITIVE CONTROL — this dispatch does publish the slot, so the zero below is a + // measurement and not a variant that was never matched. + assert_eq!( + run(true), + (true, 1), + "EmitCommittedCopyTokenEntry with the token present must publish it exactly once to \ + `last_created_token_ids`" + ); + assert_eq!( + run(false), + (true, 0), + "EmitCommittedCopyTokenEntry with the token gone must publish NOTHING: \ + `TargetFilter::LastCreated` resolves through `state.objects`, so a dead id here is a \ + \"the token you created\" reference to an object that never finished entering" + ); + } + + /// The COPY-BATCH BUFFER half of the same invariant, and the one every test above is + /// structurally blind to. + /// + /// WHY A SEPARATE TEST RATHER THAN A SIXTH ARM. Every fixture above builds + /// `GameState::new_two_player(42)`, whose resolution stack has no `ResolutionFrame::CopyToken`, + /// so `state.active_copy_token_mut()` returns `None` and the copy-batch branch these two routes + /// carry is NEVER ENTERED. Those tests fail when the ledger-3 guard is reverted, which proves + /// they are sensitive to the lines that changed — it does not prove they reach the branch where + /// the guard can be defeated. Sensitivity is not coverage, and the gap it left was real: the + /// guard shipped with an UNGUARDED `pending.created_ids.push(id)` one line below it at both + /// sites, republishing the id the guard had just withheld. + /// + /// WHAT MAKES THE BUFFER LOAD-BEARING: `token_copy::drain_copy_token_resolution` ends with + /// `state.last_created_token_ids = pending.created_ids;` — an ASSIGNMENT. So a dead id in the + /// buffer does not merely duplicate ledger 3, it OVERWRITES the guarded ledger with the + /// unguarded list. This test therefore measures the state after that drain, which is the + /// user-visible end state a `TargetFilter::LastCreated` reference actually reads. + /// + /// REACHABILITY IS DEMONSTRATED, NOT ASSERTED. The frame is seeded with a SURVIVOR id that + /// exists only in the buffer, never in ledger 3 before the drain. `survivor_rows == 1` after + /// the drain is reachable only if the `Some(pending)` branch's container was published, so + /// deleting the `if let Some(pending)` body inside + /// `token::record_last_created_copy_batch_token` fails the PRESENT row (the token's own id + /// never reaches the buffer, so it is gone after the wholesale assign) while the survivor + /// column still proves the drain itself ran. + /// + /// TWO-SIDED, each mutant failing the SAME named assertion — the `gone` row's `dead_rows == 0`: + /// * MUTANT-DROP — delete the `if !record_last_created_token(…) { return; }` early return in + /// `token::record_last_created_copy_batch_token`, i.e. the exact shape that shipped. + /// * MUTANT-TRIVIALIZE — keep every branch, replace `state.objects.contains_key(&object_id)` + /// in `token::record_last_created_token` with `true`. + /// + /// Both leave the PRESENT row passing, so neither is caught by a fixture that never ran. + #[test] + fn a_vanished_token_never_reaches_the_anaphora_slot_through_the_copy_batch_buffer() { + use crate::types::game_state::PendingCopyTokenResolution; + use std::collections::VecDeque; + + /// The two routes that publish a single just-created id while a copy batch is in flight. + enum Route { + /// `counters.rs`'s own arm. + FinalizeCopyTokenEntry, + /// Dispatched by `counters.rs` into + /// `token_copy::apply_remaining_token_modifications_after_counter_pause`. + ApplyCopyTokenModificationsAndFinalize, + } + + /// `(dead id rows in the buffer, dead id rows in ledger 3 after the drain, survivor rows in + /// ledger 3 after the drain)`. + fn run(route: &Route, present: bool) -> (usize, usize, usize) { + let mut state = GameState::new_two_player(42); + let source_id = create_object( + &mut state, + CardId(1), + PlayerId(0), + "Token Source".to_string(), + Zone::Battlefield, + ); + // An earlier batch's token. It lives ONLY in the buffer, so its presence in ledger 3 + // after the drain is proof the buffer was published there. + let survivor = create_object( + &mut state, + CardId(2), + PlayerId(0), + "Earlier Copy".to_string(), + Zone::Battlefield, + ); + let object_id = create_object( + &mut state, + CardId(0), + PlayerId(0), + "Copy Token".to_string(), + Zone::Battlefield, + ); + state.push_copy_token(PendingCopyTokenResolution { + created_ids: vec![survivor], + remaining: VecDeque::new(), + effect_kind: EffectKind::CopyTokenOf, + source_id, + }); + assert!( + state.active_copy_token().is_some(), + "REACH-GUARD: without an active CopyToken frame the branch under test is dead code \ + and every count below would be vacuous" + ); + assert!( + state.last_created_token_ids.is_empty(), + "REACH-GUARD: ledger 3 must start empty, or the rows below are not attributable to \ + this dispatch" + ); + let action = match route { + Route::FinalizeCopyTokenEntry => PendingCounterPostAction::FinalizeCopyTokenEntry { + object_id, + name: "Copy Token".to_string(), + enters_attacking: false, + source_id, + controller: PlayerId(0), + }, + Route::ApplyCopyTokenModificationsAndFinalize => { + PendingCounterPostAction::ApplyCopyTokenModificationsAndFinalize { + object_id, + name: "Copy Token".to_string(), + enters_attacking: false, + source_id, + controller: PlayerId(0), + remaining_modifications: Vec::new(), + } + } + }; + if !present { + // CR 704.5f: the live shape is a 0-toughness copy buried while its counter-ordering + // prompt was open. Removed BEFORE the resume, which is where the window is. + state.objects.remove(&object_id); + state.battlefield.retain(|id| *id != object_id); + } + let mut events = Vec::new(); + apply_pending_counter_post_action(&mut state, action, &mut events); + let buffered = state + .active_copy_token() + .expect("REACH-GUARD: the dispatch must not consume the CopyToken frame") + .created_ids + .iter() + .filter(|id| **id == object_id) + .count(); + // The frame has no remaining batches, so this drain is exactly the terminal + // `state.last_created_token_ids = pending.created_ids;` assignment and nothing else. + crate::game::effects::token_copy::drain_pending_copy_token_resolution( + &mut state, + &mut events, + ); + let rows = |wanted: ObjectId| { + state + .last_created_token_ids + .iter() + .filter(|id| **id == wanted) + .count() + }; + (buffered, rows(object_id), rows(survivor)) + } + + for (route, arm) in [ + ( + Route::FinalizeCopyTokenEntry, + "counters::FinalizeCopyTokenEntry", + ), + ( + Route::ApplyCopyTokenModificationsAndFinalize, + "counters::ApplyCopyTokenModificationsAndFinalize -> \ + token_copy::apply_remaining_token_modifications_after_counter_pause", + ), + ] { + // POSITIVE CONTROL + REACHABILITY DEMONSTRATION. The buffer column can only be 1 if the + // `Some(pending)` branch executed, and the survivor column can only be 1 if the drain + // published that buffer onto ledger 3. + assert_eq!( + run(&route, true), + (1, 1, 1), + "{arm}: with the token present it must reach the copy batch's `created_ids` AND \ + survive the drain's wholesale assignment onto `last_created_token_ids`, alongside \ + the earlier batch's token. Deleting the `if let Some(pending)` body in \ + `token::record_last_created_copy_batch_token` drops this to (0, 0, 1)" + ); + + // THE INVARIANT. + assert_eq!( + run(&route, false), + (0, 0, 1), + "{arm}: a vanished token must not enter the copy batch's `created_ids`, because \ + the drain assigns that buffer WHOLESALE onto `last_created_token_ids` — so an \ + unguarded buffer push both republishes the id ledger 3's guard withheld and \ + destroys the guarded ledger. The survivor row must stay 1: the drain still runs \ + and still publishes, which is what makes the two zeros a measurement of the guard \ + rather than of a drain that never happened" + ); + } + } } diff --git a/crates/engine/src/game/effects/gift_delivery.rs b/crates/engine/src/game/effects/gift_delivery.rs index fbc1160cc6..33a95867dc 100644 --- a/crates/engine/src/game/effects/gift_delivery.rs +++ b/crates/engine/src/game/effects/gift_delivery.rs @@ -154,29 +154,25 @@ fn create_gift_token( } crate::game::layers::mark_layers_full(state); - crate::game::restrictions::record_battlefield_entry(state, obj_id); crate::game::restrictions::record_token_created(state, obj_id); // CR 111.1 + CR 603.6a: Token creation is a zone change from outside the // game — emit `ZoneChanged { from: None }` so ETB triggers (Soul Warden, // Panharmonicon, etc.) fire for gift tokens through the normal code path. - let zone_change_record = state - .objects - .get(&obj_id) - .expect("token just created") - .snapshot_for_zone_change(obj_id, None, Zone::Battlefield); - events.push(GameEvent::ZoneChanged { - object_id: obj_id, - from: None, - to: Zone::Battlefield, - record: Box::new(zone_change_record), - }); - - events.push(GameEvent::TokenCreated { - object_id: obj_id, - name: name.to_string(), + // + // CR 400.7 + CR 608.2i + CR 603.2c: route the record and the entry pair through the single + // `from: None → Battlefield` authority so the emitted `ZoneChanged` carries this turn's real + // zone-change index instead of the `0` placeholder. The authority performs the CR 608.2i + // battlefield-entry bookkeeping itself, so the co-located `record_battlefield_entry` call is + // deleted — keeping it would double-count `battlefield_entries_this_turn`. + super::token::push_committed_token_entry_events( + state, + obj_id, + name.to_string(), source_id, - }); + events, + ) + .expect("token just created"); obj_id } diff --git a/crates/engine/src/game/effects/incubate.rs b/crates/engine/src/game/effects/incubate.rs index 78c6e7049c..6cb3bc296c 100644 --- a/crates/engine/src/game/effects/incubate.rs +++ b/crates/engine/src/game/effects/incubate.rs @@ -96,7 +96,7 @@ pub fn resolve( // token (escalates to a full pass if it sources effects, carries // counters, etc.). crate::game::layers::mark_layers_entered(state, obj_id); - // CR 403.3 battlefield-entry bookkeeping is done by `record_zone_change` below — + // CR 608.2i battlefield-entry bookkeeping is done by `record_zone_change` below — // recording it here too would double-count `battlefield_entries_this_turn`. crate::game::restrictions::record_token_created(state, obj_id); @@ -111,26 +111,16 @@ pub fn resolve( // `token.rs::apply_create_token_after_replacement_with_created_ids` and // `conjure.rs`'s identical fix for the same bug class. // - // CR 400.7 + CR 603.2c: route the record through `restrictions::record_zone_change` — the - // single authority that assigns this turn's zone-change index — and write the assigned index - // back onto the emitted record. `snapshot_for_zone_change` leaves it at its `0` placeholder, - // and the batched zone-change replay guard (`triggers.rs`) dedups on - // `(definition_ref, turn_zone_change_index)` read off the EVENT, so an unrouted record aliases - // this Incubator onto occurrence `0` and a `batched: true` ETB trigger that already fired for - // another entry this turn is swallowed. Same shape as `merge.rs` and `token.rs`. - let mut zone_change_record = state - .objects - .get(&obj_id) - .expect("incubator token was just created") - .snapshot_for_zone_change(obj_id, None, Zone::Battlefield); - zone_change_record.turn_zone_change_index = - crate::game::restrictions::record_zone_change(state, zone_change_record.clone()); - events.push(GameEvent::ZoneChanged { - object_id: obj_id, - from: None, - to: Zone::Battlefield, - record: Box::new(zone_change_record), - }); + // CR 400.7 + CR 608.2i + CR 603.2c: route the record and the emit through + // `zones::record_and_emit_entry_from_no_zone` — the single `from: None → Battlefield` + // authority, which assigns this turn's zone-change index through + // `restrictions::record_zone_change` and writes it back onto the record it emits. + // `snapshot_for_zone_change` leaves that index at its `0` placeholder, and the batched + // zone-change replay guard (`triggers.rs`) dedups on `(definition_ref, turn_zone_change_index)` + // read off the EVENT, so an unrouted record aliases this Incubator onto occurrence `0` and a + // `batched: true` ETB trigger that already fired for another entry this turn is swallowed. + crate::game::zones::record_and_emit_entry_from_no_zone(state, obj_id, events) + .expect("incubator token was just created"); super::token::inject_predefined_token_abilities(state, obj_id); diff --git a/crates/engine/src/game/effects/token.rs b/crates/engine/src/game/effects/token.rs index caaa381d6e..2b30f8200c 100644 --- a/crates/engine/src/game/effects/token.rs +++ b/crates/engine/src/game/effects/token.rs @@ -950,7 +950,7 @@ pub(crate) fn apply_create_token_after_replacement_with_created_ids( // continuous effect / carries counters / etc., or if any active effect // reads board population. crate::game::layers::mark_layers_entered(state, obj_id); - // CR 403.3 battlefield-entry bookkeeping is done by `record_zone_change` inside + // CR 608.2i battlefield-entry bookkeeping is done by `record_zone_change` inside // `push_committed_token_entry_events` below — recording it here too double-counts. crate::game::restrictions::record_token_created(state, obj_id); @@ -1134,7 +1134,7 @@ pub fn apply_resolved_token_creation( } ResolvedTokenBody::Spec { .. } => inject_resolved_token_abilities(state, object_id), } - // CR 400.7 + CR 403.3: the resolve path records the birth through + // CR 400.7 + CR 608.2i: the resolve path records the birth through // `restrictions::record_zone_change` (`push_committed_token_entry_events`), // which appends to this turn's zone-change ledger and assigns the entry's // index. Replay must record the same entry: the ledger length IS the index @@ -1681,7 +1681,7 @@ pub(crate) fn finalize_committed_liminal_token_entry_from_action( enters_attacking, attach_to, sacrifice_at, - mut created_ids, + created_ids, ability_injection, entry_events, } = action @@ -1699,7 +1699,7 @@ pub(crate) fn finalize_committed_liminal_token_entry_from_action( } } crate::game::layers::mark_layers_entered(state, object_id); - // CR 403.3 battlefield-entry bookkeeping is done by `record_zone_change`, reached from the + // CR 608.2i battlefield-entry bookkeeping is done by `record_zone_change`, reached from the // `entry_events` match below (directly on the `Emit` route, via the parked entry's flush on // the `Suppress` route) — recording it here too double-counts. crate::game::restrictions::record_token_created(state, object_id); @@ -1718,7 +1718,7 @@ pub(crate) fn finalize_committed_liminal_token_entry_from_action( }; } - // CR 400.7 + CR 403.3 + CR 614.12a: the entry RECORD and the entry EVENTS are one indivisible + // CR 400.7 + CR 608.2i + CR 614.12a: the entry RECORD and the entry EVENTS are one indivisible // operation over one snapshot, and both wait until the object IS the thing that entered. // `Emit` means it already is (nothing is deferred on that route). `Suppress` means it is not // yet — `BecomeCopy` has not run and any mandatory as-enters choice is unanswered — so the @@ -1782,14 +1782,25 @@ pub(crate) fn finalize_committed_liminal_token_entry_from_action( crate::game::triggers::install_delayed_trigger(state, sacrifice_token, events); } - created_ids.push(object_id); state.last_created_token_ids = created_ids; + // Publishing the anaphora slot goes through the guarded authority, so a token that vanished + // during the counter pause is not named by `TargetFilter::LastCreated` on the same route that + // withheld its `TokenCreated` and wrote no `created_tokens_this_turn` row. Appending after the + // assignment is the same position `created_ids.push(object_id)` produced. + record_last_created_token(state, object_id); true } -/// CR 603.6a + CR 400.7: emit a token's battlefield-entry events, recording the entry through -/// [`crate::game::restrictions::record_zone_change`] — the single authority that assigns this -/// turn's zone-change index and performs the CR 403.3 battlefield-entry bookkeeping. +/// CR 603.6a + CR 400.7 + CR 111.1: emit a token's battlefield-entry pair — the CR 400.7 zone +/// change and the CR 111.1 token creation. +/// +/// The zone-change half is delegated to +/// [`crate::game::zones::record_and_emit_entry_from_no_zone`], the single authority for a +/// `from: None → Battlefield` entry: it records the row through +/// `restrictions::record_zone_change` (which assigns this turn's zone-change index and performs +/// the CR 608.2i battlefield-entry bookkeeping) and emits the `ZoneChanged` carrying that index. +/// This function adds only the token-specific `TokenCreated` (CR 111.1), which the authority must +/// not emit because conjured cards route through it too. /// /// The index matters: `GameObject::snapshot_for_zone_change` leaves /// `turn_zone_change_index` at its `0` placeholder for the recorder to overwrite, and the @@ -1798,50 +1809,199 @@ pub(crate) fn finalize_committed_liminal_token_entry_from_action( /// therefore shipped index `0` on the wire, so a SECOND same-turn token batch collided with the /// first and its batched trigger fire was swallowed. /// -/// Callers must NOT also call `record_battlefield_entry` — `record_zone_change` does it, and a -/// second call double-counts `battlefield_entries_this_turn`. +/// Callers must NOT also call `record_battlefield_entry` — the authority's `record_zone_change` +/// does it, and a second call double-counts `battlefield_entries_this_turn`. /// /// This is the `TokenEntryEventEmission::Emit` half of the lifecycle: the object is already fully /// realized when the finalize tail runs, so record and emit happen inline. The `Suppress` half -/// parks the entry and realizes it through [`flush_pending_token_battlefield_entry`], which pairs -/// the same two authorities in the same order. +/// parks the entry and realizes it through [`flush_pending_token_battlefield_entry`], which routes +/// back through this same function. +/// +/// THE INVARIANT, enforced here rather than at call sites: **`TokenCreated` is emitted if and only +/// if the authority recorded the entry.** `record_and_emit_entry_from_no_zone` returns `None` +/// exactly when `state.objects` has no row for `object_id` (its `?` is on that one lookup and +/// nothing else — `snapshot_for_zone_change` and `record_zone_change` both return non-`Option` +/// values), so `record.is_some()` IS the object-existence predicate, read off the authority's own +/// verdict instead of a duplicated `contains_key`. +/// +/// The third token-creation ledger, `last_created_token_ids`, carries the SAME predicate through +/// [`record_last_created_token`] rather than through this function — see that doc for why the write +/// cannot be folded in here without widening the `TargetFilter::LastCreated` slot to callers that +/// deliberately do not claim it. +/// +/// Why the predicate lives HERE. `restrictions::record_token_created` — which populates +/// `created_tokens_this_turn` and `players_who_created_token_this_turn` — is itself +/// existence-guarded, so an unconditional emit puts a live trigger event +/// (`trigger_matchers::match_token_created`, keyed in `trigger_index`) on the wire with no ledger +/// row behind it. The damage is a WRONG TRIGGER FIRE, not merely a self-inconsistent read: +/// `match_token_created` applies its CR 111.2 controller filter only inside +/// `if let Some(token_controller) = state.objects.get(object_id)`, and `valid_card_matches` +/// short-circuits `None => true` without reading `state`, so with the object gone the filter is +/// never applied and the matcher returns `true` for a controller it should have rejected. +/// MEASURED: `valid_card=None, valid_target=Controller` → present `false`, gone `true`; the +/// `valid_card=Typed{Creature}` row stays `false` on the gone path and is the negative control. +/// +/// Guarding each CALLER instead was tried and abandoned: three successive enumerations of "the +/// routes where a pause can separate creation from emit" each shipped incomplete (`counters.rs` +/// x2 → then `token_copy.rs` → then the `TokenEntryEventEmission::Emit` arm reached via +/// `PendingCounterPostAction::FinalizeCommittedLiminalTokenEntry`), with +/// [`flush_pending_token_battlefield_entry`] disclosed-but-unfixed the whole time. The eight +/// callers are `apply_create_token_after_replacement_with_created_ids`, `gift_delivery.rs`, +/// `token_copy.rs` x2, `counters.rs` x2, the `Emit` arm, and that flush; this is the ONLY +/// production emit of `GameEvent::TokenCreated`, so one predicate inside it closes the class by +/// construction instead of by enumeration. See +/// `a_vanished_counter_paused_token_reports_neither_creation_event_nor_ledger_row`, which drives +/// all five deferred routes. +/// +/// KNOWN CLASS-WIDE FIX, deliberately NOT made here: `match_token_created` should resolve the +/// controller from last-known information the way its sibling `valid_card_matches_with_lki` already +/// does for the card filter. That is a change to a shared matcher affecting every gone-object +/// event, so it is a follow-up rather than part of this authority fold. +/// +/// NO CR settles whether a token that never successfully entered should fire a creation trigger, +/// because the situation is unreachable in rules terms: CR 704.3 checks state-based actions only +/// "whenever a player would get priority", so nothing can remove the token between its creation and +/// the CR 603.6a enters-the-battlefield check. The gone arm is a defensive engine artifact of +/// deferring the emit past a replacement pause, and the requirement on it is internal agreement, +/// not a rules verdict. +/// +/// Returns the recorded row (index assigned) so a caller whose object is a just-created invariant +/// can `.expect(…)` it. The return is load-bearing through the `.expect(…)` at `gift_delivery.rs` +/// and `token_copy.rs`'s uninterrupted copy path only; every other caller discards it. Those two +/// panic on `None` exactly as before — the guard changes only whether an (unobservable, because the +/// unwinding drops `events` and no engine boundary catches it) `TokenCreated` was pushed first. pub(crate) fn push_committed_token_entry_events( state: &mut GameState, object_id: ObjectId, name: String, source_id: ObjectId, events: &mut Vec, -) { - let record = record_committed_token_entry(state, object_id); - push_token_entry_events_for_record(record, object_id, name, source_id, events); +) -> Option { + let record = crate::game::zones::record_and_emit_entry_from_no_zone(state, object_id, events); + if record.is_some() { + events.push(GameEvent::TokenCreated { + object_id, + name, + source_id, + }); + } + record } -/// CR 400.7 + CR 403.3: record a token's battlefield entry through -/// [`crate::game::restrictions::record_zone_change`] — the single authority that assigns this -/// turn's zone-change index and performs the CR 403.3 battlefield-entry bookkeeping — and emit -/// NOTHING. +/// CR 111.1: Publish a just-created token into `state.last_created_token_ids`, the THIRD +/// token-creation ledger, under the same object-existence predicate as the other two. /// -/// Split out of [`push_committed_token_entry_events`] because the record is *state*, not an -/// event; both of its callers ([`push_committed_token_entry_events`] and -/// [`flush_pending_token_battlefield_entry`]) pair it with the emit in the same breath. +/// THE LEDGER TRIPLE, and why this exists. A token creation writes three places, and until this +/// function they did not agree on the object-gone path: /// -/// Returns the recorded zone change with its index assigned, so the caller emits the row it just -/// wrote instead of recording a second time (which would double-count -/// `battlefield_entries_this_turn`). `None` when the object is already gone. -pub(crate) fn record_committed_token_entry( - state: &mut GameState, - object_id: ObjectId, -) -> Option { - let mut zone_change_record = state - .objects - .get(&object_id) - .map(|token| token.snapshot_for_zone_change(object_id, None, Zone::Battlefield))?; - zone_change_record.turn_zone_change_index = - crate::game::restrictions::record_zone_change(state, zone_change_record.clone()); - Some(zone_change_record) +/// 1. `created_tokens_this_turn` — guarded, inside `restrictions::record_token_created` +/// 2. `players_who_created_token_this_turn` — guarded, same function +/// 3. `last_created_token_ids` — UNGUARDED +/// +/// THE WRITER POPULATION, NAMED BY THE QUERY THAT PRODUCED IT AND BY THE TIP IT WAS RUN AT — a +/// count with no command behind it is unfalsifiable, and this ledger has now been mis-swept twice +/// from a too-narrow grep. Counts below are MATCH EVENTS +/// (`rg --pcre2 -U --json … | grep -c '"type":"match"'`), not lines, because two of the buffer +/// writes span three lines each. With +/// `M=(push|extend|clear|insert|append|retain|splice|truncate|remove|drain|resize|pop)`: +/// +/// * `rg -n --pcre2 -U "state\s*\.\s*last_created_token_ids\s*(\.\s*M\s*\(|=[^=])" crates/engine/src` +/// → 39 at THIS tip, of which 3 are prose in comments this change added (including the one four +/// lines below), leaving 36 code hits; plus 1 more bound as `s.` (`engine.rs`'s per-turn +/// `.clear()`) = 37 = 20 production writers and 17 inside `#[cfg(test)]`. Exactly ONE production +/// writer publishes a single just-created id — the `push` in this function. The other 19 are +/// clears (4) and bulk republishes (15) of a vector that is itself a clone of this ledger, a +/// `CopyTokenApplyStatus` built one line after `.expect("token just created")`, or the copy-batch +/// buffer below. AT `4b34e5465` the same query returned 39 hits with NO comment among them, for +/// 23 production writers, of which 5 published a single just-created id. +/// * `rg -n --pcre2 -U "pending\s*\.\s*created_ids\s*(\.\s*M\s*\(|=[^=])" crates/engine/src` +/// → 11 at THIS tip, of which 1 is prose in a comment this change added (`counters.rs`'s test +/// doc), leaving 10 production writers of `PendingCopyTokenResolution::created_ids`, the +/// copy-batch buffer that `token_copy.rs`'s drain assigns WHOLESALE back onto this ledger, and 0 +/// inside `#[cfg(test)]`. Exactly ONE is a single-id publish — the `push` in +/// [`record_last_created_copy_batch_token`]. AT `4b34e5465` the same query returned 11 with no +/// comment among them, for 11 production writers, of which TWO were single-id publishes; both are +/// now that one call. +/// +/// THOSE TOTALS ARE POSITIVE CONTROLS, NOT INVARIANTS, and they churn in a specific way worth +/// naming: the query matches its own documentation, so writing prose ABOUT this ledger moves the +/// number. Both raw totals were already stale when the previous revision quoted them, invalidated +/// by the very comments that quoted them. What does NOT churn is the classification — one single-id +/// publish per container, each inside an authority — and that half is enforced executably by +/// `battlefield_entry_authority_census`'s THIRD anchor +/// (`every_single_id_anaphora_publish_lives_in_an_authority`), which pins the production multiset to +/// `{effects/token.rs: 2}` and both hits' enclosing functions to these two. Prefer running that test +/// over trusting the numbers above. +/// +/// The `-U` is load-bearing, not decoration: `token_copy.rs:321-323` and `:327-329` write +/// `pending` / `.created_ids` / `.extend(…)` across three lines, so a line-oriented grep reports 9 +/// match events where there are 11 — at BOTH tips. Deriving this population from +/// `grep 'last_created_token_ids.push('` is how round 8 found 4 of 5 sites and round 9 left the two +/// buffer siblings unguarded. +/// +/// Gating the `GameEvent::TokenCreated` emit in [`push_committed_token_entry_events`] made the +/// event agree with ledgers 1 and 2, which FLIPPED which ledger it disagreed with rather than +/// removing the disagreement: on a deferred route whose token vanished during the pause, the event +/// was withheld and both turn ledgers stayed empty while ledger 3 still held the dead id. +/// +/// Ledger 3 is not inert bookkeeping. It is the `TargetFilter::LastCreated` anaphora slot — +/// `game/filter.rs`'s `LastCreated => state.last_created_token_ids.contains(&object_id)` and +/// `game/targeting.rs`'s `LastCreated => state.last_created_token_ids.clone()` — so a dead id in it +/// is a "the token you created" reference pointing at an object that never finished entering. +/// +/// WHY NOT FOLD IT INTO `restrictions::record_token_created`, which is where the other two live: +/// MEASURED, its production call sites are a strict SUPERSET of ledger 3's (it additionally runs at +/// `incubate.rs`, `gift_delivery.rs`, `token.rs` x2, `token_copy.rs` and `counters.rs`'s +/// `InjectPredefinedTokenAbilities` arms, none of which publishes the anaphora slot), so folding +/// would silently widen `LastCreated` to routes that deliberately do not claim it. The predicate is +/// therefore single-sourced here while the call-site set stays exactly what it was. +/// +/// Not folded into [`push_committed_token_entry_events`] either, for the mirror reason: three of +/// that emitter's eight callers do not publish ledger 3, so pulling the write inside would widen +/// the slot the same way. +/// +/// Returns whether the id was published, so the copy-batch mirror in +/// [`record_last_created_copy_batch_token`] can consume the SAME verdict instead of re-deriving it. +pub(crate) fn record_last_created_token(state: &mut GameState, object_id: ObjectId) -> bool { + let exists = state.objects.contains_key(&object_id); + if exists { + state.last_created_token_ids.push(object_id); + } + exists +} + +/// CR 111.1 + CR 707.2: publish a just-created token into BOTH destinations the +/// anaphora slot has while a copy batch is in flight — [`record_last_created_token`]'s ledger 3 and +/// the in-flight `PendingCopyTokenResolution::created_ids` buffer — under ONE evaluation of the +/// object-existence predicate. +/// +/// WHY THIS EXISTS AS A FUNCTION rather than two adjacent statements. `token_copy.rs`'s drain ends +/// with `state.last_created_token_ids = pending.created_ids;` — an ASSIGNMENT, not an append. So +/// the buffer is not a secondary cache of ledger 3; it OVERWRITES it. A caller that guarded the +/// ledger write and then pushed the same id into the buffer one line below published the withheld +/// id anyway and destroyed the guarded list on top of it. That is exactly what shipped at +/// `counters.rs` and `token_copy.rs` after the guard was introduced: the predicate was single- +/// sourced but the *publish* was not, so the guard was defeated one line below itself. Fusing both +/// writes into one call leaves no second statement to forget. +/// +/// NOT folded into [`record_last_created_token`] itself, and the distinction is behavioural rather +/// than stylistic: its other three callers (`counters.rs`'s `FinalizeTokenEntry` arm and +/// `EmitCommittedCopyTokenEntry` arm, and `finalize_committed_liminal_token_entry_from_action`) +/// deliberately do NOT mirror. Mirroring there would add a plain (or already-batched) token to a +/// copy batch's `created_ids`, and since that buffer is assigned wholesale onto ledger 3 at the +/// drain, it would silently widen what `TargetFilter::LastCreated` names for "the tokens created +/// this way" — the same widening argument that keeps the predicate out of +/// `restrictions::record_token_created`. +pub(crate) fn record_last_created_copy_batch_token(state: &mut GameState, object_id: ObjectId) { + if !record_last_created_token(state, object_id) { + return; + } + if let Some(pending) = state.active_copy_token_mut() { + pending.created_ids.push(object_id); + } } -/// CR 400.7 + CR 403.3 + CR 614.12a: realize a postponed token battlefield entry — record it +/// CR 400.7 + CR 608.2i + CR 614.12a: realize a postponed token battlefield entry — record it /// through `record_zone_change` and emit its entry pair — at the first instant the object IS the /// thing that entered. Record and emit are ONE indivisible operation over ONE owned value, so no /// route can perform half of it. Returns `false` when no entry is parked for `object_id`. @@ -1850,7 +2010,7 @@ pub(crate) fn record_committed_token_entry( /// the same object is a no-op and the duplicate-row class is unrepresentable rather than guarded. /// /// LOOK-BACK WINDOW (owned, not hidden): between the commit and this flush the token is on the -/// battlefield with ZERO rows on either CR 400.7 / CR 403.3 ledger, and on a paused route that +/// battlefield with ZERO rows on either CR 400.7 / CR 608.2i ledger, and on a paused route that /// window spans one or more client round-trips. `game/quantity.rs`'s zone-change scans and /// `restrictions::battlefield_entry_matches_filter` therefore answer "0 entered this turn" for it /// during the window. That is inherent to postponing, and it is the lesser error: recording early @@ -1866,8 +2026,10 @@ pub(crate) fn record_committed_token_entry( /// the entry still parked. That is exactly why [`realize_settled_token_battlefield_entry`] is /// called from inside `apply_action` BEFORE that pipeline — a copy realized with toughness 0 gets /// its CR 400.7 row written and its pair emitted before CR 704.5f can bury it. -/// [`record_committed_token_entry`]'s `None` arm remains the fail-safe for an object that is gone -/// by flush time. +/// [`crate::game::zones::record_and_emit_entry_from_no_zone`]'s `None` arm remains the fail-safe +/// for an object that is gone by flush time: it records nothing and emits nothing, and +/// [`push_committed_token_entry_events`] now withholds `TokenCreated` on that same verdict, so this +/// route reports NOTHING rather than a creation event with no ledger row behind it. pub(crate) fn flush_pending_token_battlefield_entry( state: &mut GameState, object_id: ObjectId, @@ -1879,9 +2041,8 @@ pub(crate) fn flush_pending_token_battlefield_entry( else { return false; }; - let record = record_committed_token_entry(state, pending.object_id); - push_token_entry_events_for_record( - record, + push_committed_token_entry_events( + state, pending.object_id, pending.name, pending.source_id, @@ -1947,30 +2108,6 @@ pub(crate) fn realize_settled_token_battlefield_entry( } } -/// The event half of a token battlefield entry, shared by the immediate (`Emit`) and postponed -/// (`Suppress` + flush) routes so the emitted pair is defined exactly once. -fn push_token_entry_events_for_record( - record: Option, - object_id: ObjectId, - name: String, - source_id: ObjectId, - events: &mut Vec, -) { - if let Some(record) = record { - events.push(GameEvent::ZoneChanged { - object_id, - from: None, - to: Zone::Battlefield, - record: Box::new(record), - }); - } - events.push(GameEvent::TokenCreated { - object_id, - name, - source_id, - }); -} - // ── Layer B: token-handler batch purity gate (Tier 3) ──────────────────── /// CR 603.2 + CR 603.6a: The §2.2a emits-exactly-{ZoneChanged,TokenCreated} @@ -1996,7 +2133,7 @@ pub(crate) fn spec_emits_only_etb_pair(spec: &TokenSpec) -> bool { && spec.attach_to.is_none() // no host attachment mutation (CR 303.4) } -/// CR 603.6a + CR 111.10: The set of event keys a single produced token EMITS as +/// CR 603.6a + CR 111.1: The set of event keys a single produced token EMITS as /// it enters the battlefield, given its core types. Mirrors the event-side /// deriver exactly (`keys_from_event`, trigger_index.rs:462-468 for the ETB pair /// and :529-531 for `TokenCreated`): a token entering emits the broad @@ -2015,7 +2152,11 @@ fn produced_token_emitted_keys( .iter() .map(|ct| TriggerEventKey::EnterBattlefield(Some(*ct))), ); - // CR 111.10: a token's creation also emits `TokenCreated`. + // CR 111.1 ("Some effects put tokens onto the battlefield"): a token's + // creation also emits `TokenCreated`. NOT CR 111.10, which is the + // predefined-token characteristics catalog (Treasure/Food/Clue/Role) and + // says nothing about event emission — the ~58 other CR 111.10 citations in + // this file are correct for exactly that catalog. keys.push(TriggerEventKey::TokenCreated); keys } @@ -3901,9 +4042,9 @@ mod tests { (state, events) } - // ── CR 403.3: the entry RECORD is not gated on event emission ──────── + // ── CR 608.2i: the entry RECORD is not gated on event emission ──────── - /// CR 400.7 + CR 403.3 rows for `object_id`, as `(battlefield_entry_rows, zone_change_rows)`. + /// CR 400.7 + CR 608.2i rows for `object_id`, as `(battlefield_entry_rows, zone_change_rows)`. fn ledger_rows(state: &GameState, object_id: ObjectId) -> (usize, usize) { ( state @@ -3963,10 +4104,14 @@ mod tests { /// taken at flush. Recording here instead writes CR 400.7's "state at the moment of the move" /// from a pre-copy 0/0 Shapeshifter, which is the defect this lifecycle replaces. /// - /// REVERT-PROBE (discriminating, RUN): replace the `Suppress` park with - /// `record_committed_token_entry(state, object_id);` ⇒ the row counts here read `(1, 1)` and - /// the pending assertion fails, while `suppress_does_not_emit_the_entry_pair` below still - /// passes — isolating the flip to the record, not the events. + /// REVERT-PROBE (discriminating, RUN): replace the `Suppress` park with a pre-lifecycle + /// RECORD-ONLY inline — take `state.objects.get(&object_id)`'s + /// `snapshot_for_zone_change(object_id, None, Zone::Battlefield)` and pass it straight to + /// `restrictions::record_zone_change`, emitting nothing — ⇒ the row counts here read `(1, 1)` + /// and the pending assertion fails, while `suppress_does_not_emit_the_entry_pair` below still + /// passes — isolating the flip to the record, not the events. The substitution is deliberately + /// record-only (NOT `zones::record_and_emit_entry_from_no_zone`, which also emits): emitting + /// would break the paired isolation claim. #[test] fn suppressed_liminal_entry_parks_instead_of_recording() { let (state, object_id, _events) = @@ -4079,9 +4224,13 @@ mod tests { ); } - /// CR 704.5f fail-safe: if the object is gone when the flush runs, `record_committed_token_entry` - /// has nothing to snapshot, so no CR 400.7 row is written and no `ZoneChanged` is emitted. - /// (`TokenCreated` still reports the creation that did happen.) + /// CR 704.5f fail-safe: if the object is gone when the flush runs, + /// `zones::record_and_emit_entry_from_no_zone` has nothing to snapshot, so no CR 400.7 row is + /// written and NEITHER entry event is emitted — `push_committed_token_entry_events` gates + /// `TokenCreated` on that same `None` verdict. The class-level coherence pin for this route + /// (against the `created_tokens_this_turn` ledger, driven through + /// `apply_pending_counter_post_action`) is `counters.rs`'s + /// `a_vanished_counter_paused_token_reports_neither_creation_event_nor_ledger_row`. #[test] fn flushing_after_the_object_left_the_battlefield_records_nothing() { let (mut state, object_id, _events) = @@ -4100,10 +4249,9 @@ mod tests { "a vanished object gets no CR 400.7 row" ); assert!( - !events - .iter() - .any(|event| matches!(event, GameEvent::ZoneChanged { .. })), - "no phantom entry event is emitted; got {events:?}" + events.is_empty(), + "neither half of the entry pair is emitted for an object that is not there; \ + got {events:?}" ); } @@ -4155,8 +4303,11 @@ mod tests { ); // (iii) CR 704.5f: settled, but the token has left the battlefield ⇒ the parked entry is - // DROPPED — no row and, unlike a direct flush, no `TokenCreated` for an object that - // is not there. + // DROPPED without ever reaching the flush — no row and no events. A direct flush on + // a gone object now agrees; see + // `flushing_after_the_object_left_the_battlefield_records_nothing`. The difference + // here is only that the gate consumes the park itself and returns `false`, so there + // is no slice for the boundary convergence to scan. let (mut departed, departed_id, _events) = finalize_liminal_entry_under(TokenEntryEventEmission::Suppress); departed.battlefield.retain(|id| *id != departed_id); diff --git a/crates/engine/src/game/effects/token_copy.rs b/crates/engine/src/game/effects/token_copy.rs index 512caa7632..2ced01f18a 100644 --- a/crates/engine/src/game/effects/token_copy.rs +++ b/crates/engine/src/game/effects/token_copy.rs @@ -848,25 +848,22 @@ pub(crate) fn apply_copy_token_after_replacement_with_created_ids( // for just this token. `flush_layers` escalates to a full pass when // the copied object sources a continuous effect, carries a CDA, etc. crate::game::layers::mark_layers_entered(state, token_id); - crate::game::restrictions::record_battlefield_entry(state, token_id); crate::game::restrictions::record_token_created(state, token_id); - let zone_change_record = state - .objects - .get(&token_id) - .expect("token just created") - .snapshot_for_zone_change(token_id, None, Zone::Battlefield); - events.push(GameEvent::ZoneChanged { - object_id: token_id, - from: None, - to: Zone::Battlefield, - record: Box::new(zone_change_record), - }); - events.push(GameEvent::TokenCreated { - object_id: token_id, - name: name.clone(), + // CR 400.7 + CR 608.2i + CR 603.2c: route the record and the entry pair through the single + // `from: None → Battlefield` authority so the emitted `ZoneChanged` carries this turn's + // real zone-change index instead of the `0` placeholder. The authority performs the + // CR 608.2i battlefield-entry bookkeeping itself, so the co-located + // `record_battlefield_entry` call is deleted — keeping it would double-count + // `battlefield_entries_this_turn`. + super::token::push_committed_token_entry_events( + state, + token_id, + name.clone(), source_id, - }); + events, + ) + .expect("token just created"); created_ids.push(token_id); } @@ -966,26 +963,28 @@ pub(crate) fn apply_remaining_token_modifications_after_counter_pause( } super::token::inject_predefined_token_abilities(state, token_id); crate::game::layers::mark_layers_entered(state, token_id); - crate::game::restrictions::record_battlefield_entry(state, token_id); crate::game::restrictions::record_token_created(state, token_id); - if let Some(token) = state.objects.get(&token_id) { - let zone_change_record = token.snapshot_for_zone_change(token_id, None, Zone::Battlefield); - events.push(GameEvent::ZoneChanged { - object_id: token_id, - from: None, - to: Zone::Battlefield, - record: Box::new(zone_change_record), - }); - } - events.push(GameEvent::TokenCreated { - object_id: token_id, - name, - source_id, - }); - state.last_created_token_ids.push(token_id); - if let Some(pending) = state.active_copy_token_mut() { - pending.created_ids.push(token_id); - } + // CR 400.7 + CR 608.2i + CR 603.2c: route the record and the entry pair through the single + // `from: None → Battlefield` authority so the emitted `ZoneChanged` carries this turn's real + // zone-change index instead of the `0` placeholder. The authority performs the CR 608.2i + // battlefield-entry bookkeeping itself, so the co-located `record_battlefield_entry` call is + // deleted — keeping it would double-count `battlefield_entries_this_turn`. + // + // OBJECT-GONE: one of four counter-pause / deferred resume routes with this shape, all covered + // by the same predicate inside `push_committed_token_entry_events` — it gates `TokenCreated` on + // the authority's `None` verdict, so a vanished token cannot put a live creation event on the + // wire with no `created_tokens_this_turn` row behind it. MEASURED with the predicate deleted, + // token removed: `(TokenCreated=1, created_tokens_this_turn=0, last_created_token_ids=1)` — + // exactly the disagreement + // `a_vanished_counter_paused_token_reports_neither_creation_event_nor_ledger_row` forbids. The + // anaphora slot below carries the SAME predicate through + // `record_last_created_copy_batch_token`, which is the third ledger of the triple; without it + // the gone path reads `(0, 0, 1)`. That call owns BOTH of the slot's destinations — ledger 3 + // and this batch's `created_ids`, which `drain_pending_copy_token_resolution` assigns wholesale + // back onto ledger 3 — because a separate buffer push republished the withheld id and clobbered + // the guarded list on top of it. + super::token::push_committed_token_entry_events(state, token_id, name, source_id, events); + super::token::record_last_created_copy_batch_token(state, token_id); true } @@ -1857,7 +1856,7 @@ mod tests { assert!(events.iter().any( |e| matches!(e, GameEvent::TokenCreated { name, .. } if name == "Mist-Syndicate Naga") )); - // Verify record_battlefield_entry and record_token_created were called + // Verify record_token_created was called assert!( state .players_who_created_token_this_turn diff --git a/crates/engine/src/game/zones.rs b/crates/engine/src/game/zones.rs index a7be581206..b842105e49 100644 --- a/crates/engine/src/game/zones.rs +++ b/crates/engine/src/game/zones.rs @@ -1337,6 +1337,75 @@ pub fn move_to_zone( }); } +/// CR 400.7 + CR 608.2i + CR 603.6a: record AND emit the battlefield entry of an object that came +/// into existence on the battlefield — a zone change with NO origin zone (`from: None`): a created +/// token (CR 111.1), a copy token (CR 707.2), an Incubator, or a conjured card. The `Some(from)` +/// counterpart is the emit at the end of `move_to_zone`. +/// +/// Routes through [`crate::game::restrictions::record_zone_change`] — the single authority that +/// assigns this turn's zone-change index and performs the CR 608.2i battlefield-entry bookkeeping — +/// then writes the assigned index back onto the record it emits. +/// +/// Callers must NOT also call `restrictions::record_battlefield_entry` (`record_zone_change` does +/// it; a second call double-counts `battlefield_entries_this_turn`) and must NOT also push onto +/// `state.zone_changes_this_turn` (that would write a duplicate CR 400.7 row). +/// +/// WHY record and emit are ONE call: `GameObject::snapshot_for_zone_change` leaves +/// `turn_zone_change_index` at its `0` placeholder for the recorder to overwrite. The CR 603.2c +/// batched zone-change replay guard (`triggers.rs::batched_zone_change_already_collected`) dedups +/// on `(definition_ref, turn_zone_change_index)` read off the EVENT, and +/// `Ability::self_ref_own_departure_successor` (`types/ability.rs`) uses that same index as a +/// SUBSCRIPT into `state.zone_changes_this_turn`, then requires the row it lands on to carry the +/// same `trigger_source_context().identity.reference` as the event's own record. An entry that +/// emits without recording therefore ships index `0`, aliases onto occurrence `0`, and both +/// consumers read a row belonging to a different object. Splitting the two halves is what made +/// that defect writable at SIX call sites (measured on `4b34e5465`: `conjure.rs`, `counters.rs` x2, +/// `gift_delivery.rs`, `token_copy.rs` x2); fusing them removes the seam a seventh would be written +/// through. +/// +/// Tripwired — not proved impossible — by +/// `crates/engine/tests/integration/battlefield_entry_authority_census.rs`, a source-text census +/// whose ceilings are documented in its own module header. +/// +/// Returns the recorded row with its assigned index. `None` when the object is gone, in which case +/// NOTHING is recorded and NOTHING is emitted. +/// +/// THE `None` ARM IS NOT A SILENT NO-OP AT EVERY CALLER, and an earlier revision of this paragraph +/// said it was — it named `gift_delivery.rs` and `token_copy.rs`, which are callers of +/// [`crate::game::effects::token::push_committed_token_entry_events`] ONE LEVEL UP, not of this +/// function. (That sentence is correct about ITS subject: of that emitter's eight callers, exactly +/// those two `.expect(…)` its return.) Measured over this function's four direct callers with +/// `rg -n 'record_and_emit_entry_from_no_zone\(' crates/engine/src`: +/// +/// * `effects/conjure.rs:218` — `.expect("conjured object was just created")`: PANICS on `None`. +/// * `effects/incubate.rs:123` — `.expect("incubator token was just created")`: PANICS on `None`. +/// * `effects/token.rs:1881` — `if record.is_some()`, which is how +/// `push_committed_token_entry_events` gates its `GameEvent::TokenCreated` emit. This is the +/// object-existence predicate the token-creation ledger triple agrees on. +/// * `effects/counters.rs:530` — statement position, discards. +/// +/// So `None` is inert on exactly ONE of the four routes. The two `.expect` callers keep their +/// pre-existing "just created" panic deliberately: each creates its object inside the same call, so +/// `None` there is an engine invariant violation rather than a reachable game state. +pub(crate) fn record_and_emit_entry_from_no_zone( + state: &mut GameState, + object_id: ObjectId, + events: &mut Vec, +) -> Option { + let mut record = state + .objects + .get(&object_id) + .map(|obj| obj.snapshot_for_zone_change(object_id, None, Zone::Battlefield))?; + record.turn_zone_change_index = super::restrictions::record_zone_change(state, record.clone()); + events.push(GameEvent::ZoneChanged { + object_id, + from: None, + to: Zone::Battlefield, + record: Box::new(record.clone()), + }); + Some(record) +} + /// CR 601.2 + CR 733.1: Restore an object while reversing an incomplete action. /// This intentionally uses the raw mover rather than the replacement-consulting /// pipeline: an undone action does not apply replacement effects, but preserves @@ -2065,9 +2134,14 @@ pub(crate) fn apply_battlefield_entry_controller_override( .expect("resolved controller override must have a live journal cause"); } -/// Retags the CR 400.7 zone-change and CR 403.3 battlefield-entry snapshots at +/// Retags the CR 400.7 zone-change and CR 608.2i battlefield-entry snapshots at /// the exact recorded positions. Shared by the resolve-time authority and the /// replay applier so both install the same retag. +/// +/// CR 608.2i, not CR 403.3: `battlefield_entries_this_turn` is an entry-time +/// characteristics snapshot kept so later effects can look back at a previous +/// game state. CR 403.3 ("Permanents exist only on the battlefield") is +/// definitional and describes no such record. fn retag_battlefield_entry_snapshots( state: &mut GameState, zone_change_index: Option, diff --git a/crates/engine/tests/integration/battlefield_entry_authority_census.rs b/crates/engine/tests/integration/battlefield_entry_authority_census.rs new file mode 100644 index 0000000000..a939585c7c --- /dev/null +++ b/crates/engine/tests/integration/battlefield_entry_authority_census.rs @@ -0,0 +1,1889 @@ +//! STRUCTURAL CENSUS over the FUSED battlefield-entry event pair and the anaphora slot behind it. +//! Three anchors, one instrument: +//! +//! * `GameEvent::ZoneChanged` — every production construction in `crates/engine/src` whose `from` +//! field carries NO origin zone (spelled `from: None` or written in field-init shorthand) must +//! live inside `zones::record_and_emit_entry_from_no_zone`, the single +//! `from: None → Battlefield` record+emit authority, except three sites adjudicated BY NAME. +//! * `GameEvent::TokenCreated` — every production construction must live inside +//! `token::push_committed_token_entry_events`, the single emitter, except one site adjudicated +//! BY NAME. See the SECOND ANCHOR section below. +//! * SINGLE-ID PUBLISHES into either anaphora container (`state.last_created_token_ids` and +//! `PendingCopyTokenResolution::created_ids`) — every production one must live inside +//! `token::record_last_created_token` or `token::record_last_created_copy_batch_token`. No +//! adjudicated survivors. See the THIRD ANCHOR section below. +//! +//! The first two anchors share [`literal_body`] and [`classify_anchor`]; all three share +//! `cfg_test_scoped_lines`, [`rs_files`] and the [`top_level_fn_headers`] / [`enclosing_fn`] scope +//! resolver. A second copy of that machinery would drift away from this one. +//! +//! CR 400.7 + CR 608.2i + CR 603.2c: `GameObject::snapshot_for_zone_change` leaves +//! `turn_zone_change_index` at a `0` placeholder for `restrictions::record_zone_change` to +//! overwrite. An entry that emits its `ZoneChanged` without reaching the recorder ships that +//! placeholder, so the CR 603.2c batched zone-change replay guard aliases it onto occurrence `0` +//! and `Ability::self_ref_own_departure_successor` subscripts a ledger row belonging to a DIFFERENT +//! object. +//! +//! COUNT, MEASURED on the pre-change tree (`b654513cb`), not recalled: SIX production writers had +//! the defect — `conjure.rs:213`, `counters.rs:535`, `counters.rs:839`, `gift_delivery.rs:168`, +//! `token_copy.rs:859`, `token_copy.rs:973`. Two more sites wrote the same +//! `from: None → Battlefield` shape but already routed through the recorder (`incubate.rs:128`, +//! `token.rs:1960`), for eight writers total. Fusing record and emit removes the call-site seam +//! the six were written through, and this census is the tripwire for a SEVENTH. +//! +//! WHAT THIS DOES NOT CLAIM. It is a SOURCE-TEXT instrument, so it is a tripwire against an +//! accidental clone, NOT a proof that one cannot be written. Three measured ceilings, all of them +//! real: +//! +//! * It cannot see a construction assembled through a helper that takes `from` as a parameter, +//! nor one built by a macro. Neither exists today: the only non-`GameEvent::`-prefixed +//! `ZoneChanged {` occurrences in `engine/src` are comments and the enum declaration in +//! `types/events.rs`. +//! * A construction written ENTIRELY in field-init shorthand +//! (`GameEvent::ZoneChanged { object_id, from, to, record }`) is textually identical to a +//! destructuring pattern, so [`is_construction`] cannot separate them and the census skips it. +//! This is the one FAIL-OPEN ceiling, and it is kept deliberately. The second anchor's +//! [`is_token_construction`] closes the same ceiling by keying on FIELD COMPLETENESS, and +//! backporting that predicate here was MEASURED rather than assumed: over `engine/src` it takes +//! this anchor from the pinned 4 production / 10 test hits to 16 / 13, and every one of the 15 +//! additions is a destructuring CONSUMER (`let … else`, `if let`, match arms in +//! `types/game_state.rs` x4, `game/triggers.rs` x2, `trigger_matchers.rs`, `trigger_index.rs`, +//! `filter.rs`, `derived_views.rs`, `visibility.rs`, `merge_tests.rs`) — zero are constructions. +//! The asymmetry is empirical, not structural: `TokenCreated`'s three fields are never all bound +//! by a consumer (the widest elides one, `{ object_id, source_id, .. }`), while `ZoneChanged`'s +//! four are routinely all bound because consumers need every one of them. So the mirror-image +//! ceiling is fail-CLOSED in direction but not in magnitude — it would replace the pin with a +//! 12-file consumer list that churns on every unrelated edit, which is the exact outcome arm +//! 4(iii) exists to prevent. The same sweep found NO all-shorthand `ZoneChanged` construction in +//! `engine/src`, so the ceiling is currently unexercised. +//! * Conversely, a PATTERN that renames a field (`record: rec`) reads as a construction and would +//! be counted. That direction is fail-CLOSED — it can only ADD an unexpected hit and fail the +//! exact multiset below. No such pattern exists in `engine/src` today (measured). +//! +//! STRUCTURAL COMPLETENESS, ENFORCED BY STRUCTURE — NEVER BY SUBSTRING. Round 3 of this census +//! keyed on the substring `from: None` inside a fixed six-line window from the anchor. Both halves +//! were evadable, and both evasions COMPILE (each is now a permanent arm of the anti-vacuity test +//! below, forms 6 and 7): +//! +//! 1. `let from = None;` followed by field-init shorthand `from,` — the value is still `None`, but +//! the substring `from: None` never appears. +//! 2. Writing the `record:` field first as a multi-line `Box::new(…)` expression, which pushes +//! `from: None` past the sixth line. +//! +//! Both are closed by scanning the literal's OWN extent (brace depth, [`literal_body`]) and by +//! classifying the `from` FIELD rather than matching a spelling of it. +//! +//! A THIRD THROUGH SIXTH evasion of the same instrument were found in rounds 8, 10 and 12, and +//! unlike the first two they are FAIL-OPEN — they REMOVE a hit rather than adding one: +//! +//! 3. An ordinary prose comment inside the literal carrying an unbalanced `}`. The brace scan +//! counted it as the literal's closing brace and returned a TRUNCATED body, so every field +//! written after the comment went unseen. This also defeated arm 4(ii), whose comment is +//! brace-free and so never reached the scan. +//! 4. The same truncation through a `}` inside a string literal. +//! 5. The same truncation through a NESTED block comment. Rust block comments nest; the scan +//! tracked them with a BOOL, so `/* outer /* inner */ } */` left comment state at the inner +//! `*/` and the `}` truncated the body. This one survived the rewrite that closed 3 and 4, and +//! the residual list that rewrite shipped did not name it. +//! 6. The same truncation through a RAW BYTE / RAW C string (`br#"…"#`, `cr#"…"#`). `b` and `c` +//! are alphanumeric, so the guard that stops an identifier's trailing `r` opening a raw string +//! rejected these too, and the `"` after it fell through to the `#`-blind escaped-string scan. +//! The residual list that closed 5 did NOT name this one either, because that list said it was +//! "derived from the branches the scanner actually has" — and a branch the scanner does not +//! have is exactly what such a derivation cannot see. [`literal_body`]'s list is therefore now +//! derived from an EXTERNAL basis: the Rust Reference's enumeration of tokens whose interior +//! text is not code, each mapped to the branch that consumes it. +//! +//! All four are closed in [`literal_body`] — comment/string/char skipping for 3 and 4, a comment +//! DEPTH counter for 5, [`literal_prefix_start`] for 6 — and all four are permanent arms (1d, and +//! 4(iv) for the second anchor), each paired with the SAME literal carrying a balanced comment, a +//! non-nested comment, or an unprefixed raw string as its control, so each pair measures its +//! specific evasion rather than comments or raw strings in general. +//! +//! SEPARATELY, TWO MEASURED CEILINGS in the reused `cfg_test_scoped_lines` scope classifier (these +//! are about test/production SCOPING, not about finding the literal), both FAIL-CLOSED here: +//! +//! 1. Its rule is `opens_module || next.trim_end().ends_with('{')`, so a `#[cfg(test)]` item whose +//! `fn` signature spans multiple lines (header ending in `(`) is never scoped. Live instance: +//! `trigger_matchers.rs`'s `#[cfg(test)] pub(crate) fn test_trigger_source_context(`. +//! 2. It matches only the literal string `#[cfg(test)]`, so +//! `#[cfg(any(test, feature = "test-support"))]` is never scoped either. Live instance: +//! `ability.rs`'s `set_test_trigger_source_recursive`. +//! +//! Both are fail-CLOSED for THIS census because CONJUNCT 1 pins the production hit set as an EXACT +//! per-file multiset in which absent files must not appear: a mis-scoped test hit can only ADD an +//! unexpected file/count and FAIL, never let a real clone through. Fixing the classifier would move +//! `loop_shortcut_offer_writer_census`'s own pinned numbers, which is out of this change's scope. +//! Arm 1 of the anti-vacuity test MEASURES ceiling 1 rather than asserting it. +//! +//! ── SECOND ANCHOR: `GameEvent::TokenCreated` ───────────────────────────────────────────────── +//! +//! WHY IT EXISTS. The `ZoneChanged` anchor above pins only HALF of the fused pair. The other half +//! is the one this change moved: the `TokenCreated` emit was hoisted out of eight call sites into +//! `token::push_committed_token_entry_events` and gated on the recorder's own verdict. Until this +//! anchor existed, that half was protected by nothing but the fact that exactly one production +//! emit site happens to exist today — i.e. by ENUMERATION, the instrument the change argues +//! against. A new `events.push(GameEvent::TokenCreated { … })` written anywhere else tripped +//! nothing. +//! +//! COUNT, MEASURED on this tree, not recalled: 38 anchor occurrences in `engine/src`, of which +//! exactly TWO are production-scope constructions — the emitter (`effects/token.rs`, inside +//! `push_committed_token_entry_events`) and the adjudicated `stack.rs` probe. The other 10 +//! constructions are `#[cfg(test)]`-scoped and the remaining 26 are consumer PATTERNS. +//! +//! THE CONSTRUCTION PREDICATE IS DIFFERENT HERE, AND THAT DIFFERENCE IS THE POINT. `ZoneChanged` +//! keys on an explicit `record:` initializer, so it cannot see a construction written entirely in +//! field-init shorthand — that is ceiling 2 above. The token emitter IS written entirely in +//! shorthand (`{ object_id, name, source_id }`), so reusing that predicate would have made this +//! anchor blind to the one site it exists to pin. [`is_token_construction`] therefore keys on +//! FIELD COMPLETENESS, which is Rust's own rule rather than a spelling: a struct-variant +//! EXPRESSION must initialise every field (enum variants admit no `..base` functional update), +//! while a PATTERN may elide any of them with `..`. Naming all three of +//! [`TOKEN_CREATED_FIELDS`] therefore counts every construction with NO false negatives. +//! +//! Its one ceiling is the mirror image and is fail-CLOSED: an EXHAUSTIVE pattern that binds all +//! three fields (`GameEvent::TokenCreated { object_id, name, source_id }`) is textually identical +//! to a construction and would be counted, ADDING an unexpected hit to the exact multiset below. +//! MEASURED: no such pattern exists in `engine/src` today — every consumer elides at least one +//! field, the widest being `effects/destroy.rs`'s `{ object_id, source_id, .. }`. +//! +//! The other ceilings carry over verbatim, for the same reasons and in the same direction: +//! +//! * A construction assembled through a helper taking the fields as parameters, or built by a +//! macro, is invisible. Neither exists today: the only non-`GameEvent::`-prefixed +//! `TokenCreated {` occurrence in `engine/src` is the enum declaration in `types/events.rs`. +//! * `cfg_test_scoped_lines`'s two scope ceilings are fail-CLOSED here for exactly the reason +//! they are above — a mis-scoped test hit can only ADD an unexpected file to the per-file +//! production multiset and FAIL, never let a real clone through. +//! * COMPLETENESS ACROSS CRATES, MEASURED: `GameEvent::TokenCreated` occurs in no crate outside +//! `crates/engine/`. The four occurrences under `crates/engine/tests/**` are acceptance-row +//! consumers of this surface, not members of it, and that tree is deliberately not walked. +//! +//! RESIDUAL, STATED RATHER THAN IMPLIED: the function-scope conjunct pins only the `token.rs` +//! hit. A second production construction written inside `stack.rs` that ALSO deleted the probe +//! would keep the multiset intact and pass. That is the same residual the `ZoneChanged` half +//! carries for each of its three non-authority files; closing it would mean a per-file enclosing-fn +//! pin on files whose hits are adjudicated survivors rather than authorities. +//! +//! ── THIRD ANCHOR: the SINGLE-ID ANAPHORA PUBLISH ───────────────────────────────────────────── +//! +//! WHY IT EXISTS. The two anchors above pin the two halves of the entry EVENT. The defect that +//! actually shipped in this change's round 9 and round 10 was neither: it was a single just-created +//! id published into the `TargetFilter::LastCreated` anaphora slot WITHOUT the object-existence +//! predicate — first into `state.last_created_token_ids` directly, then, after that was guarded, +//! into `PendingCopyTokenResolution::created_ids` one line BELOW the guard, which the copy-batch +//! drain assigns wholesale back onto ledger 3 and so republished the id the guard had withheld. +//! Four successive enumeration sweeps each declared that class closed. Its closure then rested on a +//! query written in a doc comment, i.e. on the same ENUMERATION this file's other two anchors exist +//! to replace. This anchor is that query made executable. +//! +//! WHAT IT KEYS ON, and why it inherits none of [`literal_body`]'s residuals: a method CALL, not a +//! struct-literal body. There is no brace scan, no comment/string/char skipping and no +//! construction-vs-pattern predicate — the anchor is the container's FIELD ACCESS and the verdict +//! is the next code token after it ([`call_tail`], then [`is_single_id_publish`] or +//! [`is_ambiguous_mutator`]). The FOUR fail-OPEN truncations that rounds 8, 10 and 12 found in +//! [`literal_body`] — evasions 3 through 6 — are therefore structurally absent here rather than +//! argued absent. (Four truncations, three rounds: round 12's byte-raw and raw-C prefixes are one +//! evasion with two spellings, and an earlier revision of this sentence counted the ROUNDS.) +//! +//! THE TAIL VOCABULARY, MEASURED on this tree rather than assumed — every distinct next-token form +//! following `.last_created_token_ids` or `.created_ids` in `engine/src`, over 157 field accesses: +//! `=` 39 (bulk assign), `[` 26 (indexed read), `.clone(` 25, `.len(` 19, `.is_empty(` 12, +//! `.iter(` 8, `;` 5, `{` 5 (a `for … in &state.last_created_token_ids {` header), `.extend(` 5, +//! `.contains(` 3, `.first(` 3, `)` 3, `.push(` 2, `.clear(` 1, `,` 1. `.push(` occurs exactly +//! TWICE, both inside the two authorities. NO tail begins with `/*`, which is what makes the +//! block-comment residual below latent rather than live. +//! +//! WHAT THE CLASSIFIER DOES WITH THAT VOCABULARY IS NOT DERIVED FROM IT — the derivation basis is +//! `std`'s method semantics, for the same reason [`literal_body`]'s residual list had to stop being +//! derived from the branches the scanner happened to have. `.push(` / `.insert(` always introduce +//! exactly one element; `.extend(` / `.append(` / `.resize(` / `.splice(` introduce one only for +//! arguments this scanner cannot read, so they are PINNED by conjunct 4 rather than judged; +//! `.clear(` / `.retain(` / `.truncate(` / `.remove(` / `.drain(` / `.pop(` cannot introduce an +//! element at all. `.insert(`, `.append(`, `.resize(` and `.splice(` occur zero times today and are +//! covered anyway — a classifier that only knew the spellings present in the tree is a classifier +//! that closes nothing. See [`is_single_id_publish`] and [`is_ambiguous_mutator`]. +//! +//! THE RESIDUALS, all fail-OPEN unless noted, each measured: +//! +//! * A publish through an ALIAS (`let v = &mut state.last_created_token_ids; v.push(id);`) is +//! invisible: the alias's own `.push` carries neither container name. The same query covers the +//! UFCS form `Vec::push(&mut state.last_created_token_ids, id)`, whose tail is a bare `,`. +//! MEASURED absent — +//! `rg -n --pcre2 -U '&\s*mut\s+\w+\.(last_created_token_ids|created_ids)' crates/engine/src` +//! exits 1 with no output, against a positive control of 89 match events for +//! `&\s*mut\s+\w+\.objects` under the identical query shape. This is the direct analogue of the +//! other two anchors' helper/macro ceiling. +//! * An INDEXED ASSIGNMENT (`state.last_created_token_ids[0] = id;`) introduces an id under a tail +//! this anchor reads as `[`, which is also the 26 indexed READS. Deliberately NOT classified: +//! separating them needs bracket matching plus a lookahead past the `]`, i.e. the fixed-window +//! guess [`literal_body`]'s doc rejects, and counting `[` wholesale would move the conjunct-1 pin +//! from `(2, 0)` to `(3, 25)`, its production multiset from `{effects/token.rs: 2}` to +//! `{database/encore_tests.rs: 1, effects/token.rs: 2}`, and the files it touches from one to +//! seven — 25 of the 26 `[` tails are `#[cfg(test)]` fixtures reading `…_ids[0]`, so the pin +//! would churn on every new test that reads the ledger. MEASURED absent instead: +//! `rg -n --pcre2 -U '\.(last_created_token_ids|created_ids)\s*\[[^\]]*\]\s*=[^=]' +//! crates/engine/src` exits 1 with no output, against a positive control of 211 match events for +//! `\w\s*\[[^\]]*\]\s*=[^=]` under the identical query shape. +//! * A block comment BETWEEN the field and its method call is a tail [`call_tail`] cannot read. +//! Made fail-CLOSED by counting it — see [`is_single_id_publish`], arm 5(vi). +//! * PROSE that quotes the defect is skipped only in the three shapes [`code_span`] and the +//! full-line rule handle: a full-line `//`, a leading `/* … */`, and a trailing `//` not preceded +//! by a `"`. A `//` after a quote, a `/* … */` opened mid-line, a string literal containing the +//! container-plus-verb text, and the interior lines of a multi-line block comment are still +//! scanned. That residue is fail-CLOSED — it can only ADD a hit and fail the exact multiset, +//! never hide a publish. MEASURED over every OCCURRENCE rather than every matching line: of 184 +//! container-name occurrences in `engine/src`, 27 sit in a full-line `//` and 157 are ordinary +//! code — the same 157 the tail vocabulary above enumerates — and ZERO sit in any of the four +//! residue shapes today. +//! * A publish into a container reached by a name this anchor does not know is invisible. The +//! container set is closed by type rather than by search: `TargetFilter::LastCreated` reads +//! `state.last_created_token_ids` (`game/filter.rs`, `game/targeting.rs`) and the only other +//! writer of that field is the copy-batch drain's wholesale assign from +//! `PendingCopyTokenResolution::created_ids`. Those are the two names. + +use std::path::Path; + +use super::loop_shortcut_offer_writer_census::{cfg_test_scoped_lines, rs_files}; + +/// The bare anchor, ASSEMBLED AT RUNTIME so this file can never count its own text — the doctrine +/// `loop_shortcut_offer_writer_census` files against its own superseded round-2 anchor. +/// +/// Deliberately BARE: no `to:` qualifier. A qualifier-anchored detector is evadable by spelling +/// (`to: dest`), which is exactly the evasion the round-2 anchor lost to. Destination specificity +/// is CONJUNCT 3's job, as a secondary. +fn anchor() -> String { + format!("{}::{} {{", "GameEvent", "ZoneChanged") +} + +/// The second bare anchor, assembled at runtime for the same self-counting reason. +/// +/// Deliberately BARE for the same reason as [`anchor`]: any field qualifier is evadable by +/// spelling, and the emitter writes all three of its fields in shorthand anyway. +fn token_anchor() -> String { + format!("{}::{} {{", "GameEvent", "TokenCreated") +} + +/// The literal's OWN extent: everything between the anchor's opening `{` and its matching `}`. +/// +/// This replaces a fixed-size line window. A window is a guess about how long a literal is, and a +/// literal is exactly as long as its braces say — writing the `record:` field first as a multi-line +/// `Box::new(…)` expression pushes `from: None` off the end of any fixed window while leaving the +/// construction intact (evasion 2, form 7 below). +/// +/// Nested braces are tracked, so `record: Box::new(ZoneChangeRecord { … })` does not terminate the +/// scan early. Braces that are NOT code are SKIPPED rather than counted: `//` and `/* … */` +/// comments (block comments by DEPTH, because Rust nests them), and string / raw-string / char +/// literals. Counting them was a FAIL-OPEN defect — a `}` in an ordinary prose comment closed the +/// literal early and TRUNCATED the captured body, so every field written after it went unseen and +/// the construction scored zero. Arms 1d and 4(iv) are that evasion, each paired with the same +/// literal carrying a BALANCED comment as its control. +/// +/// WHERE THIS LIST COMES FROM, because its provenance is the thing that kept failing. Revision 1 +/// claimed the only failure mode "can only ADD hits and fail the exact multiset" — the fail-CLOSED +/// direction — and asserting it as the ONLY one is what let the fail-OPEN truncation above hide. +/// Revision 2 listed four residuals and silently omitted the NESTED block comment, inside the very +/// rewrite that closed the others. Revision 3 said it was "derived from the branches the scanner +/// actually has", which cannot work: a branch the scanner does NOT have is invisible to that +/// derivation, and round 12 duly found a fourth truncation (`br#"…"#`) sitting in the gap. +/// +/// So the derivation basis is now EXTERNAL to this scanner: the Rust Reference's list of tokens +/// whose interior text is not code, enumerated in full and each mapped to the branch that consumes +/// it. What follows is that enumeration, not a list of holes someone happened to find. +/// +/// | Rust token kind | example | consumed by | +/// |------------------------|------------|------------------------------------------------------| +/// | line comment (`//`, `///`, `//!`) | `// }` | `pair_at(_, '/', '/')` → break to end of line | +/// | block comment (NESTS; `/** */`, `/*! */`) | `/* /* } */ */` | `block_depth` | +/// | char literal | `'}'` | [`skip_literal`]'s two `'\''` arms | +/// | byte literal | `b'}'` | same arms; the `b` is emitted as ordinary body text | +/// | string | `"}"` | [`skip_escaped`] | +/// | byte string | `b"}"` | [`skip_escaped`]; the `b` is ordinary body text | +/// | C string | `c"}"` | [`skip_escaped`]; the `c` is ordinary body text | +/// | raw string | `r#"}"#` | [`skip_raw`], any `#` count | +/// | raw byte string | `br#"}"#` | [`skip_raw`] via [`literal_prefix_start`] (evasion 6) | +/// | raw C string | `cr#"}"#` | [`skip_raw`] via [`literal_prefix_start`] (evasion 6) | +/// | lifetime / loop label | `&'a str` | DELIBERATELY not a literal — see the residual below | +/// +/// That table is the completeness claim, and it is checkable by reading it against the Reference +/// rather than by trusting this file. `br` / `cr` are MEASURED absent from `engine/src` today +/// (`rg --pcre2 -U --json '(? /tmp/pc/nested.rs` then the same +/// query over `/tmp/pc` → 1. The LINE-ORIENTED query an earlier revision shipped +/// (`rg -n --pcre2 '/\*.*/\*'`) returns 0 on that same file, i.e. it was blind to the very +/// hazard it claimed to have measured absent. Arm 1d / evasion 5 pins the fix, with a NON-nested +/// block comment carrying the same stray `}` as its control. +/// * [`classify_anchor`] scans each line for the FIRST occurrence of ITS needle only, so two +/// constructions of the SAME anchor on ONE source line score 1. Fail-OPEN. MEASURED absent: +/// `rg --pcre2 -U --multiline-dotall --json 'GameEvent::ZoneChanged \{[^\n]*GameEvent::ZoneChanged +/// \{|GameEvent::TokenCreated \{[^\n]*GameEvent::TokenCreated \{' crates/engine/src` → 0 match +/// events, against a positive control proving the query CAN hit — the mixed-anchor form returns 1 +/// (`effects/token.rs`'s `ZoneChanged { .. } | TokenCreated { .. }` match arm; no line number, +/// because a cited line number churns on every edit above it), which is +/// harmless because the two anchors are scanned in separate passes. rustfmt splitting long +/// literals is why the same-anchor case stays empty rather than being argued impossible. +fn literal_body(lines: &[&str], n: usize, needle: &str) -> String { + let mut body = String::new(); + let start = lines[n] + .find(needle) + .expect("the caller located this needle on this line"); + let mut chars: Vec = lines[n][start + needle.len()..].chars().collect(); + let mut depth = 1usize; + let mut row = n; + // A DEPTH, not a bool: Rust block comments NEST, so `/* outer /* inner */ } */` leaves comment + // state at the INNER `*/` under a boolean and then counts the `}` as the literal's own closing + // brace — the same fail-OPEN truncation the comment/string skipping above closes, surviving in + // the rewrite that closed the others. Arm 1d / evasion 5 pins it. + let mut block_depth = 0usize; + loop { + let mut at = 0usize; + while at < chars.len() { + if block_depth > 0 { + if pair_at(&chars, at, '*', '/') { + block_depth -= 1; + at += 2; + } else if pair_at(&chars, at, '/', '*') { + block_depth += 1; + at += 2; + } else { + at += 1; + } + continue; + } + if pair_at(&chars, at, '/', '/') { + break; + } + if pair_at(&chars, at, '/', '*') { + block_depth += 1; + at += 2; + continue; + } + if let Some(past) = skip_literal(&chars, at) { + at = past; + continue; + } + match chars[at] { + '{' => depth += 1, + '}' => { + depth -= 1; + if depth == 0 { + return body; + } + } + _ => {} + } + body.push(chars[at]); + at += 1; + } + row += 1; + match lines.get(row) { + Some(next) => { + body.push('\n'); + chars = next.chars().collect(); + } + None => return body, + } + } +} + +/// Do `chars[at]` and its successor spell the two-character token `first``second`? +fn pair_at(chars: &[char], at: usize, first: char, second: char) -> bool { + chars[at] == first && chars.get(at + 1) == Some(&second) +} + +/// The index just past the string / raw-string / char literal starting at `at`, or `None` when no +/// literal starts there. +/// +/// The `b` / `c` PREFIXES need no arm of their own: `b"…"`, `c"…"` and `b'…'` reach the `"` / `'` +/// arms below with the prefix already emitted as ordinary body text, which is harmless because a +/// prefix carries no brace. The RAW forms are the exception and are handled by +/// [`literal_prefix_start`] — see evasion 6. +fn skip_literal(chars: &[char], at: usize) -> Option { + match chars[at] { + '"' => Some(skip_escaped(chars, at + 1)), + // `r"…"` / `r#"…"#` and the `br` / `cr` raw byte / raw C forms, but not the trailing `r` + // of an identifier. A raw IDENTIFIER (`r#foo`) falls through: its `#` run is not followed + // by a `"`, so the `.then(…)` below yields `None`. + 'r' if !literal_prefix_start(chars, at) + .checked_sub(1) + .is_some_and(|before| chars[before].is_alphanumeric() || chars[before] == '_') => + { + let hashes = chars[at + 1..].iter().take_while(|ch| **ch == '#').count(); + (chars.get(at + 1 + hashes) == Some(&'"')) + .then(|| skip_raw(chars, at + 2 + hashes, hashes)) + } + // A lifetime is not a literal, so `'` counts only in the unambiguous one-character forms. + '\'' if chars.get(at + 1) == Some(&'\\') && chars.get(at + 3) == Some(&'\'') => { + Some(at + 4) + } + '\'' if chars.get(at + 1) != Some(&'\\') && chars.get(at + 2) == Some(&'\'') => { + Some(at + 3) + } + _ => None, + } +} + +/// Where a raw string's PREFIX starts: `at` itself, or the `b` / `c` immediately before it +/// (`br"…"`, `cr"…"`, and their `#`-delimited forms). +/// +/// EVASION 6, and the reason this is a function rather than an inline condition. `b` and `c` are +/// alphanumeric, so [`skip_literal`]'s identifier guard — which exists to stop an identifier's +/// trailing `r` opening a raw string — rejected every raw BYTE and raw C string. The following `"` +/// then fell through to [`skip_escaped`], which honours no `#` delimiter and stops at the first +/// embedded `"`, so the rest of the raw string was scanned as CODE and its `}` closed the literal +/// early. Fail-OPEN, the same truncation class as evasions 3, 4 and 5: `br#"" }"#` scored (0, 0) +/// while the byte-for-byte identical `r#"" }"#` scored (1, 1). Arm 1d / evasion 6 pins the pair. +fn literal_prefix_start(chars: &[char], at: usize) -> usize { + match at.checked_sub(1) { + Some(before) if matches!(chars[before], 'b' | 'c') => before, + _ => at, + } +} + +/// Index just past the closing `"`, honouring backslash escapes. An unterminated string consumes +/// the rest of the line (residual 2 in [`literal_body`]'s doc). +fn skip_escaped(chars: &[char], from: usize) -> usize { + let mut at = from; + while at < chars.len() { + match chars[at] { + '\\' => at += 2, + '"' => return at + 1, + _ => at += 1, + } + } + chars.len() +} + +/// Index just past a raw string's `"` plus its `hashes` closing `#`s. Raw strings have no escapes, +/// which is exactly why they need their own scan: `r#"}"#` is one `}` that is not code. +fn skip_raw(chars: &[char], from: usize, hashes: usize) -> usize { + let mut at = from; + while at < chars.len() { + if chars[at] == '"' && chars[at + 1..].iter().take_while(|ch| **ch == '#').count() >= hashes + { + return at + 1 + hashes; + } + at += 1; + } + chars.len() +} + +/// Does `body` name `from` in FIELD-INIT SHORTHAND rather than with an explicit value? +/// +/// Splitting on the field separators (`,`) and on brace/newline boundaries yields one entry per +/// field; a shorthand field is the bare token. `from: None` yields `from: None`, which is not the +/// bare token, so the two spellings are classified independently and both are counted. +fn has_from_shorthand(body: &str) -> bool { + body.split([',', '{', '}', '\n']) + .any(|field| field.trim() == "from") +} + +/// Is `body` a struct EXPRESSION (a writer) rather than a destructuring PATTERN (a consumer)? +/// +/// MEASURED discriminator: an explicit `record:` initializer. Every construction in `engine/src` +/// writes `record: Box::new(…)`, and every pattern binds `record` (or `object_id`, or `to`) as a +/// bare name. Only needed for the shorthand branch — `from: None` is already unambiguous enough to +/// be pinned by name, and `log.rs`'s adjudicated PATTERN survivor is counted through it. +/// +/// The two directions are not symmetric, on purpose. A renaming pattern (`record: rec`) would be +/// misread as a construction and ADD a hit, which the exact multiset rejects — fail-closed. An +/// all-shorthand construction is invisible; that ceiling is stated in the module header. +fn is_construction(body: &str) -> bool { + body.contains("record:") +} + +/// The three fields `GameEvent::TokenCreated` declares (`types/events.rs`). +const TOKEN_CREATED_FIELDS: [&str; 3] = ["object_id", "name", "source_id"]; + +/// Is `body` a `TokenCreated` struct EXPRESSION (a writer) rather than a destructuring PATTERN? +/// +/// Keys on FIELD COMPLETENESS, not on a spelling, because the emitter writes every field in +/// shorthand and [`is_construction`]'s `record:` discriminator would score it zero. Rust requires +/// an enum struct-variant expression to initialise every field and admits no `..base` functional +/// update, so "names all three" has no false negatives; the mirror-image ceiling (an exhaustive +/// pattern binding all three) is fail-CLOSED and is stated in the module header. +/// +/// Field names are taken as the text before the first `:` of each `,`/brace/newline-delimited +/// segment, which reads `object_id: PROBE_ID` and bare `object_id` identically. +fn is_token_construction(body: &str) -> bool { + let named: Vec<&str> = body + .split([',', '{', '}', '\n']) + .map(|field| field.split(':').next().unwrap_or_default().trim()) + .collect(); + TOKEN_CREATED_FIELDS + .iter() + .all(|field| named.contains(field)) +} + +/// One classified hit. +#[derive(Debug, Clone, PartialEq, Eq)] +struct Hit { + file: String, + line: usize, + in_test: bool, + /// CONJUNCT 3's secondary: does the same window name the battlefield literally? Meaningful + /// only for the `ZoneChanged` anchor — a `TokenCreated` literal has no `to:` field, so this is + /// uniformly `false` there and no token conjunct reads it. + to_battlefield: bool, +} + +/// Locate every non-comment `needle` line in `src`, take the literal's OWN extent, and keep the +/// hits whose body satisfies `keep`. +/// +/// The shared walker for BOTH anchors. Keeping one copy is what guarantees the two pins agree on +/// the comment rule, on the brace scan, and on the `cfg_test_scoped_lines` scope resolver. +fn classify_anchor(src: &str, file: &str, needle: &str, keep: impl Fn(&str) -> bool) -> Vec { + let scoped = cfg_test_scoped_lines(src); + let lines: Vec<&str> = src.lines().collect(); + lines + .iter() + .enumerate() + .filter(|(_, line)| line.contains(needle) && !line.trim_start().starts_with("//")) + .filter_map(|(n, _)| { + let body = literal_body(&lines, n, needle); + keep(&body).then(|| Hit { + file: file.to_string(), + line: n + 1, + in_test: scoped[n], + to_battlefield: body.contains("to: Zone::Battlefield"), + }) + }) + .collect() +} + +/// Classify every no-origin-zone anchor hit in `src`. +/// +/// A hit is an anchor whose literal body either writes `from: None` outright, or names `from` in +/// field-init shorthand inside something [`is_construction`] recognises as a writer. The second +/// branch is what a spelling-based detector misses: `let from = None;` + `from,` constructs exactly +/// the same event. +/// +/// The comment-line exclusion (`!line.trim_start().starts_with("//")`) is reused verbatim from +/// `loop_shortcut_offer_writer_census::classify`'s measured rule: a comment writes no event, and a +/// comment-blind anchor makes the tripwire fire on prose. +fn classify(src: &str, file: &str) -> Vec { + classify_anchor(src, file, &anchor(), |body| { + body.contains("from: None") || (is_construction(body) && has_from_shorthand(body)) + }) +} + +/// Classify every `GameEvent::TokenCreated` CONSTRUCTION in `src`. +/// +/// Unlike [`classify`] there is no value predicate to apply: `TokenCreated` carries no origin-zone +/// field, so every construction of it is in scope and the only question is construction-vs-pattern. +fn classify_tokens(src: &str, file: &str) -> Vec { + classify_anchor(src, file, &token_anchor(), is_token_construction) +} + +/// The scan root, shared by both anchors: `crates/engine/src`, recursively, `.rs` only. +/// +/// COMPLETENESS MEASURED, not assumed: NEITHER `GameEvent::ZoneChanged` nor +/// `GameEvent::TokenCreated` occurs in any crate outside `crates/engine/`. +/// `crates/engine/tests/**` is deliberately not walked — the acceptance rows are consumers of this +/// surface, not members of it (and this very file's runtime-assembled anchors would otherwise be a +/// finding). +fn census_with(classifier: impl Fn(&str, &str) -> Vec) -> Vec { + let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut hits = Vec::new(); + for path in rs_files(&root) { + let src = std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("read {path:?}: {e}")); + let rel = path + .strip_prefix(&root) + .expect("walked path is under its root") + .to_string_lossy() + .replace('\\', "/"); + hits.extend(classifier(&src, &format!("engine/src/{rel}"))); + } + hits +} + +fn census() -> Vec { + census_with(classify) +} + +/// The `GameEvent::TokenCreated` half of the pair, over the same root and with the same exclusions. +fn token_census() -> Vec { + census_with(classify_tokens) +} + +/// The two containers the CR 111.1 anaphora slot (`TargetFilter::LastCreated`) is read out of: +/// `GameState::last_created_token_ids` (ledger 3) and `PendingCopyTokenResolution::created_ids` +/// (the copy-batch buffer, which `token_copy.rs`'s drain assigns WHOLESALE back onto ledger 3). +/// +/// Anchored on the FIELD ACCESS (leading `.`), not the bare name, so a local `created_ids` vector +/// being built up for a later bulk assignment — the deliberately-out-of-scope republish class — is +/// not confused with a write to the buffer field. +const ANAPHORA_CONTAINERS: [&str; 2] = [".last_created_token_ids", ".created_ids"]; + +/// The next code token after a container field access, skipping whitespace, line breaks and `//` +/// comments. +/// +/// THIS IS NOT A LINE SCAN, and that is the whole point. `token_copy.rs:321-323` and `:327-329` +/// write `pending` / `.created_ids` / `.extend(…)` across THREE lines, so a line-oriented rule +/// reports 9 buffer writers where there are 11 — the exact query-shape error that made round 8 +/// find 4 of 5 ledger-3 sites and left round 9's two buffer siblings unguarded. Arm 5(ii) is that +/// multi-line shape made executable. +fn call_tail(lines: &[&str], row: usize, from: usize) -> String { + let mut row = row; + let mut rest: &str = &lines[row][from..]; + loop { + let trimmed = rest.trim_start(); + if !trimmed.is_empty() && !trimmed.starts_with("//") { + return trimmed.chars().take(24).collect(); + } + row += 1; + match lines.get(row) { + Some(next) => rest = next, + None => return String::new(), + } + } +} + +/// Does `tail` publish a SINGLE id into the container it follows? +/// +/// The twelve `Vec` mutators this change's ledger-3 and buffer queries enumerate split three ways +/// by ARGUMENT-FREE semantics — which is the point, because that basis is checkable against the +/// `std` docs rather than against this tree's current contents: +/// +/// * ALWAYS introduces exactly one element, whatever its argument: `.push(`, `.insert(`. Counted +/// here, and these two only. +/// * MAY introduce one element, depending on an argument this scanner cannot read: `.extend(`, +/// `.append(`, `.resize(`, `.splice(`. `state.last_created_token_ids.extend(iter::once(id))` +/// publishes exactly one id and is indistinguishable BY TEXT from `.extend(other_vec)`. NOT +/// counted here — and NOT thereby declared bulk. [`is_ambiguous_mutator`] carries them, and +/// conjunct 4 pins their whole population, so a new one cannot land unread. +/// * CANNOT introduce an element at all: `.clear(`, `.retain(`, `.truncate(`, `.remove(`, +/// `.drain(`, `.pop(`. Excluded because removal-only is a property of the method, not because +/// they happen to be absent here. +/// +/// So the two classifiers together are the executable form of that twelve-verb query, rather than +/// a two-verb proxy for it: 2 counted, 4 pinned, 6 excluded by semantics. +/// +/// A `/* … */` tail is counted TOO, and deliberately. [`call_tail`] skips `//` comments but not +/// block comments, so a block comment written between the field and its method call would hide the +/// call from this classifier — fail-OPEN. Counting the unreadable tail instead makes it ADD a hit +/// and FAIL the exact multiset, which is the same fail-CLOSED discipline +/// [`FN_PREFIX_ALLOW_SET`]'s abort applies to an unrecognised `fn` prefix. Measured: zero of the +/// container field accesses in `engine/src` carry such a tail today. +fn is_single_id_publish(tail: &str) -> bool { + tail.starts_with(".push(") || tail.starts_with(".insert(") || tail.starts_with("/*") +} + +/// The mutators whose ARGUMENT decides whether they introduce an id, bounded by PINNING them. +/// +/// Not classified, PINNED. Conjunct 4 fixes their exact production multiset, so writing +/// `v.extend(std::iter::once(id))` anywhere in `engine/src` turns the census red and a human reads +/// the argument. That is the same trade the `/* … */` tail gets in [`is_single_id_publish`] — an +/// unreadable thing is COUNTED rather than skipped — applied to an unreadable ARGUMENT instead of +/// an unreadable tail. +/// +/// SCOPE, STATED HONESTLY BECAUSE AN EARLIER REVISION OVERSTATED IT: this is a VOCABULARY, not a +/// proof of exhaustiveness over `std`. It bounds the eight verbs listed below and nothing else. An +/// earlier revision called this "the one fail-OPEN this anchor would otherwise carry" and claimed a +/// new ambiguous publish "cannot arrive without a human reading its argument"; both were false +/// universals — `extend_from_slice`, `clone_from`, `resize_with` and `extend_from_within` each +/// compile, each publish exactly one id, and each scored `(0, 0)` under the four-verb set. They are +/// listed now. Any `std` mutator NOT listed is a named fail-OPEN of this anchor, recorded in the +/// residual list rather than denied here. +/// +/// Several of these occur zero times on either container today. They are listed because the CLASS +/// is what is being closed: "absent from the tree" is a liveness statement, not a coverage +/// statement — the lesson the `br` / `cr` raw-string evasion already paid for. +fn is_ambiguous_mutator(tail: &str) -> bool { + [ + ".extend(", + ".extend_from_slice(", + ".extend_from_within(", + ".append(", + ".resize(", + ".resize_with(", + ".splice(", + ".clone_from(", + ] + .iter() + .any(|verb| tail.starts_with(verb)) +} + +/// The byte range of `line` that is CODE: a LEADING `/* … */` comment and a TRAILING `//` comment +/// are excluded from the container search. +/// +/// Both exclusions only ever REMOVE text, and each is guarded so that it cannot remove code: +/// +/// * The leading form fires only when the TRIMMED line starts with `/*`, and drops exactly up to +/// and including the first `*/` — so `/*count=*/ state.last_created_token_ids.push(id)` keeps its +/// call (arm 5(viii)). +/// * The trailing form fires only when no `"` precedes the `//` in the remaining code — so +/// `let u = "http://x"; state.last_created_token_ids.push(id);` keeps its call (arm 5(viii)). +/// +/// The offsets are returned rather than a substring because [`call_tail`] indexes back into the +/// ORIGINAL line, and because the `/* … */` tail form of arm 5(vi) must still reach the classifier. +/// +/// Whatever survives both guards — a `//` after a quote, a `/* … */` opened mid-line, a string +/// literal quoting the defect, or the interior lines of a multi-line block comment — is still +/// scanned, and would ADD a hit. That direction is fail-CLOSED (spurious red, never a missed +/// publish); it is listed in this file's residuals. What is closed here are the two shapes this +/// change's own prose is most likely to take. +fn code_span(line: &str) -> (usize, usize) { + let trimmed = line.trim_start(); + let mut lo = 0usize; + if trimmed.starts_with("/*") { + let start = line.len() - trimmed.len(); + lo = match line[start..].find("*/") { + Some(end) => start + end + 2, + None => line.len(), + }; + } + let mut hi = line.len(); + if let Some(slash) = line[lo..].find("//") { + if !line[lo..lo + slash].contains('"') { + hi = lo + slash; + } + } + (lo, hi) +} + +/// Classify every access to either anaphora container in `src` whose TAIL satisfies `keep`. +/// +/// The third anchor's walker, parameterised by the tail predicate so that +/// [`is_single_id_publish`] and [`is_ambiguous_mutator`] walk the SAME surface and cannot drift +/// apart — the two populations partition the mutator vocabulary and a second copy of this loop +/// would be the place that stopped being true. +/// +/// It shares `cfg_test_scoped_lines`, [`rs_files`] and the [`top_level_fn_headers`] / +/// [`enclosing_fn`] resolver with the other two anchors, but deliberately NOT [`literal_body`]: +/// this anchor keys on a method CALL rather than on a struct-literal body, so it inherits none of +/// that scanner's brace/string/raw-string residuals. [`code_span`] is the one comment rule it does +/// carry, and it is subtractive-only by construction. +fn classify_container_tails(src: &str, file: &str, keep: fn(&str) -> bool) -> Vec { + let scoped = cfg_test_scoped_lines(src); + let lines: Vec<&str> = src.lines().collect(); + let mut hits = Vec::new(); + for (n, line) in lines.iter().enumerate() { + if line.trim_start().starts_with("//") { + continue; + } + let (lo, hi) = code_span(line); + let code = &line[lo..hi]; + for needle in ANAPHORA_CONTAINERS { + let mut from = 0usize; + while let Some(at) = code[from..].find(needle) { + from += at + needle.len(); + if keep(&call_tail(&lines, n, lo + from)) { + hits.push(Hit { + file: file.to_string(), + line: n + 1, + in_test: scoped[n], + // No `to:` field on a method call; no conjunct of this anchor reads it. + to_battlefield: false, + }); + } + } + } + } + hits +} + +/// Classify every SINGLE-ID publish into either anaphora container in `src` — conjuncts 1–3. +fn classify_publishes(src: &str, file: &str) -> Vec { + classify_container_tails(src, file, is_single_id_publish) +} + +/// Classify every ARGUMENT-AMBIGUOUS mutator call on either container in `src` — conjunct 4. +fn classify_ambiguous_mutators(src: &str, file: &str) -> Vec { + classify_container_tails(src, file, is_ambiguous_mutator) +} + +/// Read `crates/engine/src/` — the source both anchors resolve enclosing functions against. +fn engine_src(rel: &str) -> String { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join("src").join(rel); + std::fs::read_to_string(&path).unwrap_or_else(|e| panic!("read {path:?}: {e}")) +} + +/// The per-file production multiset of `hits`, sorted — CONJUNCT 1's shape for both anchors. +fn production_multiset(hits: &[&Hit]) -> Vec<(String, usize)> { + let mut multiset: Vec<(String, usize)> = Vec::new(); + for hit in hits { + match multiset.iter_mut().find(|(file, _)| *file == hit.file) { + Some((_, count)) => *count += 1, + None => multiset.push((hit.file.clone(), 1)), + } + } + multiset.sort(); + multiset +} + +/// Every column-0 `fn` header prefix that exists in `crates/engine/src`. +/// +/// PINNED AS AN ALLOW-SET so an UNRECOGNISED prefix (a future `const fn`, `async fn`, `unsafe fn`) +/// ABORTS the census instead of being silently skipped. A hard-coded include-list fails OPEN — the +/// unknown header is skipped and an EARLIER header wins the resolution, which is the paid-for +/// lesson recorded in +/// `replacement.rs::every_applying_path_reaches_the_recorder_because_the_hook_is_in_pipeline_loop`. +/// This fails CLOSED. +/// +/// MEASURED over all of `engine/src`, not over the two files this resolver happens to read. AT +/// THIS COMMIT'S TREE, by the command below (which implements exactly the rule +/// [`top_level_fn_headers`] applies — column 0, not a `//` line, some whitespace-delimited token is +/// a bare `fn`, prefix = the tokens before it): +/// +/// ```text +/// find crates/engine/src -name '*.rs' -exec awk ' +/// /^[ \t]/{next} /^[ \t]*\/\//{next} +/// {for(i=1;i<=NF;i++) if($i=="fn"){p="";for(j=1;j Option<…> {`) carries no bare `fn` token, so it is not collected. +/// +/// `Err` carries the extend-the-allow-set message for an unrecognised prefix. +fn top_level_fn_headers(src: &str) -> Result, String> { + let mut out = Vec::new(); + for (n, line) in src.lines().enumerate() { + if line.starts_with([' ', '\t']) || line.trim_start().starts_with("//") { + continue; + } + let tokens: Vec<&str> = line.split_whitespace().collect(); + let Some(at) = tokens.iter().position(|token| *token == "fn") else { + continue; + }; + let prefix = tokens[..at] + .iter() + .map(|token| format!("{token} ")) + .collect::(); + if !FN_PREFIX_ALLOW_SET.contains(&prefix.as_str()) { + return Err(format!( + "line {}: unrecognised top-level `fn` prefix {prefix:?} — the census resolves a \ + hit's enclosing function by scanning back to the latest column-0 header, and an \ + unknown prefix would silently resolve to an EARLIER function. EXTEND \ + `FN_PREFIX_ALLOW_SET` (currently {FN_PREFIX_ALLOW_SET:?}) and re-check that the \ + resolution below is still correct.", + n + 1 + )); + } + let name = tokens[at + 1] + .split(['(', '<']) + .next() + .unwrap_or_default() + .to_string(); + out.push((n + 1, name)); + } + Ok(out) +} + +/// The enclosing top-level `fn` of `line`: the LATEST collected header at or before it. +fn enclosing_fn(headers: &[(usize, String)], line: usize) -> Option<&str> { + headers + .iter() + .rev() + .find(|(at, _)| *at <= line) + .map(|(_, name)| name.as_str()) +} + +/// The three production `from: None` constructions that are NOT the authority, each adjudicated by +/// name with its verdict. A failure message naming them reads as "a NEW construction appeared", +/// not as "someone re-measured". +const ADJUDICATED_SURVIVORS: &str = "\n \ + engine/src/game/log.rs — a match PATTERN (`GameEvent::ZoneChanged { .., from: None, to, .. } => …`), a CONSUMER, not a writer;\n \ + engine/src/game/merge.rs — already routed through `record_zone_change`, and its `to: dest` is a VARIABLE, which the \ + `Battlefield`-hardcoding authority cannot serve;\n \ + engine/src/game/stack.rs — a synthetic `PROBE_ID` record inside `observers_are_batch_safe`, which never reaches the trigger collector.\n"; + +#[test] +fn every_from_none_battlefield_entry_construction_lives_in_the_authority() { + let hits = census(); + let production: Vec<&Hit> = hits.iter().filter(|hit| !hit.in_test).collect(); + let test_scoped: Vec<&Hit> = hits.iter().filter(|hit| hit.in_test).collect(); + + // ── CONJUNCT 1 — the surface, pinned BIDIRECTIONALLY and by per-file multiset. ─────────── + let multiset = production_multiset(&production); + + assert_eq!( + (production.len(), test_scoped.len()), + (4, 10), + "the no-origin-zone battlefield-entry construction surface moved. Expected 4 production \ + constructions — the authority plus three adjudicated survivors:{ADJUDICATED_SURVIVORS}\ + A NEW production hit means a SEVENTH clone of the record/emit split was written: route it \ + through `zones::record_and_emit_entry_from_no_zone` instead. A REMOVED hit means the \ + authority or a survivor moved. The 10 test-scoped hits are 7 spelled `from: None` plus 3 \ + written in field-init shorthand (`analysis/sim.rs`, `trigger_matchers.rs`, \ + `targeting.rs`), which the shorthand branch of `classify` is what sees. \ + Production hits = {production:#?}" + ); + + assert_eq!( + multiset, + vec![ + ("engine/src/game/log.rs".to_string(), 1), + ("engine/src/game/merge.rs".to_string(), 1), + ("engine/src/game/stack.rs".to_string(), 1), + ("engine/src/game/zones.rs".to_string(), 1), + ], + "per-file production multiset moved. Absent files must NOT appear — that exactness is what \ + makes the classifier's two known ceilings fail-CLOSED (a mis-scoped test hit can only add \ + an unexpected file and fail).{ADJUDICATED_SURVIVORS}" + ); + + // ── CONJUNCT 2 — the FUNCTION-SCOPE anchor, fail-closed by construction. ───────────────── + // + // Without this, a seventh clone written INSIDE `zones.rs` that also removed the authority's own + // hit would keep the multiset at `("zones.rs", 1)` and pass. + let zones = engine_src("game/zones.rs"); + // No assertion on `headers.len()`: the list is recomputed every run and the enclosing fn is + // resolved BY NAME below, so a count pin protects nothing and would fail on any unrelated + // top-level `fn` added to or removed from this hot, multi-agent file. + let headers = top_level_fn_headers(&zones).unwrap_or_else(|message| panic!("{message}")); + + let zones_hit = production + .iter() + .find(|hit| hit.file == "engine/src/game/zones.rs") + .expect("zones.rs carries the authority's construction"); + assert_eq!( + enclosing_fn(&headers, zones_hit.line), + Some("record_and_emit_entry_from_no_zone"), + "zones.rs's `from: None` construction at line {} is NOT inside the authority. Every such \ + construction in this file must live in `record_and_emit_entry_from_no_zone`; a second one \ + elsewhere in the file is the clone this census exists to catch.", + zones_hit.line + ); + + // ── CONJUNCT 3 — `→ Battlefield` specificity, as a SECONDARY. ──────────────────────────── + // + // Secondary because on its own it is evadable by spelling; necessary because the multiset + // alone would not fire if `merge.rs` later spelled its `dest` as a literal `Zone::Battlefield`. + let mut battlefield_files: Vec<&str> = production + .iter() + .filter(|hit| hit.to_battlefield) + .map(|hit| hit.file.as_str()) + .collect(); + battlefield_files.sort_unstable(); + assert_eq!( + battlefield_files, + vec!["engine/src/game/stack.rs", "engine/src/game/zones.rs"], + "exactly two production windows may name `to: Zone::Battlefield` literally — the named \ + `stack.rs` probe and the authority. `log.rs` binds a bare `to,` in a pattern and \ + `merge.rs` uses the variable `dest`; either of them gaining the literal is a new \ + battlefield-entry writer.{ADJUDICATED_SURVIVORS}" + ); +} + +/// The ONE production `GameEvent::TokenCreated` construction that is not the emitter, adjudicated +/// by name with its verdict — the same probe the `ZoneChanged` anchor adjudicates, which is what +/// makes it a shared verdict rather than two independent judgement calls. +const ADJUDICATED_TOKEN_SURVIVOR: &str = "\n \ + engine/src/game/stack.rs — a synthetic `PROBE_ID` construction inside `observers_are_batch_safe`, \ + handed to `trigger_index::candidates_for_event` to shape-test observers and never pushed onto a \ + real event stream, so no consumer and no ledger ever sees it.\n"; + +#[test] +fn every_token_created_construction_lives_in_the_single_emitter() { + let hits = token_census(); + let production: Vec<&Hit> = hits.iter().filter(|hit| !hit.in_test).collect(); + let test_scoped: Vec<&Hit> = hits.iter().filter(|hit| hit.in_test).collect(); + + // ── CONJUNCT 1 — the surface, pinned BIDIRECTIONALLY and by per-file multiset. ─────────── + let multiset = production_multiset(&production); + + assert_eq!( + (production.len(), test_scoped.len()), + (2, 10), + "the `TokenCreated` construction surface moved. Expected 2 production constructions — \ + `token::push_committed_token_entry_events`, the SINGLE emitter every one of its eight \ + callers inherits its `record.is_some()` gate from, plus one adjudicated \ + survivor:{ADJUDICATED_TOKEN_SURVIVOR}\ + A NEW production hit means a SECOND emit site was written: a `TokenCreated` that does not \ + go through the emitter is a creation event with no `created_tokens_this_turn` row behind \ + it, which makes `trigger_matchers::match_token_created` skip its CR 111.2 controller \ + filter and fire for a controller it should have rejected. Route it through \ + `token::push_committed_token_entry_events` instead. A REMOVED hit means the emitter or the \ + probe moved. The 10 test-scoped constructions are `analysis/sim.rs`, `game/log.rs`, \ + `game/triggers.rs` x4 and `game/effects/destroy.rs` x4. \ + Production hits = {production:#?}" + ); + + assert_eq!( + multiset, + vec![ + ("engine/src/game/effects/token.rs".to_string(), 1), + ("engine/src/game/stack.rs".to_string(), 1), + ], + "per-file production `TokenCreated` multiset moved. Absent files must NOT appear — that \ + exactness is what makes `cfg_test_scoped_lines`'s two known scope ceilings, and \ + `is_token_construction`'s exhaustive-pattern ceiling, fail-CLOSED: each can only add an \ + unexpected file and fail.{ADJUDICATED_TOKEN_SURVIVOR}" + ); + + // ── CONJUNCT 2 — the FUNCTION-SCOPE anchor, fail-closed by construction. ───────────────── + // + // Without this, a second emit written INSIDE `token.rs` that also removed the emitter's own + // construction would keep the multiset at `("effects/token.rs", 1)` and pass. `token.rs` is + // exactly where such a clone would be written, which is why this conjunct is not optional. + let token_src = engine_src("game/effects/token.rs"); + let headers = top_level_fn_headers(&token_src).unwrap_or_else(|message| panic!("{message}")); + + let token_hit = production + .iter() + .find(|hit| hit.file == "engine/src/game/effects/token.rs") + .expect("effects/token.rs carries the emitter's construction"); + assert_eq!( + enclosing_fn(&headers, token_hit.line), + Some("push_committed_token_entry_events"), + "token.rs's `GameEvent::TokenCreated` construction at line {} is NOT inside the emitter. \ + Every such construction in this file must live in `push_committed_token_entry_events`, \ + where the `record.is_some()` gate ties the event to the CR 400.7 ledger row; a second one \ + elsewhere in the file is the clone this census exists to catch.", + token_hit.line + ); +} + +/// The two authorities every single-id anaphora publish must live inside, named with what each +/// one owns, so a failure reads as "a third publisher appeared" rather than "someone re-measured". +const ANAPHORA_AUTHORITIES: &str = "\n \ + token::record_last_created_token — the object-existence predicate, and the ONLY \ + production `.push` into ledger 3;\n \ + token::record_last_created_copy_batch_token — consumes that same verdict and mirrors the id \ + into the in-flight copy batch, so one evaluation owns BOTH destinations.\n"; + +#[test] +fn every_single_id_anaphora_publish_lives_in_an_authority() { + let hits = census_with(classify_publishes); + let production: Vec<&Hit> = hits.iter().filter(|hit| !hit.in_test).collect(); + let test_scoped: Vec<&Hit> = hits.iter().filter(|hit| hit.in_test).collect(); + + // ── CONJUNCT 1 — the surface, pinned BIDIRECTIONALLY and by per-file multiset. ─────────── + let multiset = production_multiset(&production); + + assert_eq!( + (production.len(), test_scoped.len()), + (2, 0), + "the single-id anaphora-publish surface moved. Expected exactly 2 production publishes and \ + 0 test-scoped ones — the two authorities:{ANAPHORA_AUTHORITIES}\ + A NEW production hit means a single just-created id can reach \ + `TargetFilter::LastCreated` WITHOUT the object-existence predicate, which is the defect \ + this change's round 10 review found after four enumeration sweeps had each declared the \ + class closed: a token that vanished during a replacement pause stays in the anaphora slot \ + and \"the token you created\" resolves to an object that never finished entering. Route it \ + through one of the two authorities instead. A REMOVED hit means an authority moved. \ + Production hits = {production:#?}" + ); + + assert_eq!( + multiset, + vec![("engine/src/game/effects/token.rs".to_string(), 2)], + "per-file single-id anaphora-publish multiset moved. Absent files must NOT appear: \ + `counters.rs` and `token_copy.rs` are exactly where the round 9 and round 10 defects were \ + written, and their absence here is the claim this conjunct exists to keep \ + true.{ANAPHORA_AUTHORITIES}" + ); + + // ── CONJUNCT 2 — the FUNCTION-SCOPE anchor, fail-closed by construction. ───────────────── + // + // Without this, a third publish written INSIDE `token.rs` that also deleted one authority's own + // `push` would keep the multiset at `("effects/token.rs", 2)` and pass. `token.rs` is where + // both authorities live, so it is exactly where such a clone would be written. + let token_src = engine_src("game/effects/token.rs"); + let headers = top_level_fn_headers(&token_src).unwrap_or_else(|message| panic!("{message}")); + + let mut enclosing: Vec<&str> = production + .iter() + .filter(|hit| hit.file == "engine/src/game/effects/token.rs") + .map(|hit| { + enclosing_fn(&headers, hit.line).unwrap_or_else(|| { + panic!( + "token.rs publish at line {} resolves to no top-level fn", + hit.line + ) + }) + }) + .collect(); + enclosing.sort_unstable(); + assert_eq!( + enclosing, + vec![ + "record_last_created_copy_batch_token", + "record_last_created_token", + ], + "token.rs's single-id anaphora publishes are not the two authorities' own. Every `.push` \ + into either container in this file must live in one of them; a third one elsewhere in the \ + file — or one of these two moving out — is the clone this anchor exists to \ + catch.{ANAPHORA_AUTHORITIES}" + ); + + // ── CONJUNCT 4 — the ARGUMENT-AMBIGUOUS mutators, which is what makes conjuncts 1-3 a claim + // about the CLASS rather than about two spellings of it. ─────────────────────────────── + // + // `state.last_created_token_ids.extend(std::iter::once(id))` publishes exactly one id and + // reads, BY TEXT, exactly like `.extend(other_vec)`. No text scanner can tell them apart, so + // this conjunct does not try: it pins the whole `.extend(` / `.append(` / `.resize(` / + // `.splice(` population instead, and a new one anywhere in `engine/src` turns this red so a + // human reads the argument. Kept as a separate pin rather than folded into conjunct 1 for a + // measured reason: folding would take the pin to `(7, 0)` and put `counters.rs` and + // `token_copy.rs` — whose ABSENCE is conjunct 1's whole claim — back into the multiset, and + // would add a non-authority function to conjunct 2's list. + let ambiguous = census_with(classify_ambiguous_mutators); + let ambiguous_production: Vec<&Hit> = ambiguous.iter().filter(|hit| !hit.in_test).collect(); + + assert_eq!( + ( + ambiguous_production.len(), + ambiguous.len() - ambiguous_production.len() + ), + (5, 0), + "the argument-ambiguous mutator surface on the two anaphora containers moved. These are \ + the calls whose SINGLE-ID-ness this census cannot read, so they are pinned instead of \ + classified. If the new call's argument is a bulk source (a `Vec`, a \ + `CopyTokenApplyStatus`'s `created_ids`, a clone of ledger 3), update this pin in the same \ + commit. If it is a single id, it belongs in an authority instead:{ANAPHORA_AUTHORITIES}\ + Ambiguous hits = {ambiguous_production:#?}" + ); + + assert_eq!( + production_multiset(&ambiguous_production), + vec![ + ("engine/src/game/effects/counters.rs".to_string(), 2), + ("engine/src/game/effects/token.rs".to_string(), 1), + ("engine/src/game/effects/token_copy.rs".to_string(), 2), + ], + "per-file argument-ambiguous mutator multiset moved. All five of these are bulk \ + republishes today — `.extend(status.created_ids)` and `.extend(state.\ + last_created_token_ids.clone())` — and the pin exists so that a SIXTH cannot arrive \ + without someone reading its argument" + ); +} + +// ── ANTI-VACUITY ───────────────────────────────────────────────────────────────────────────── +// +// A census that cannot fail is worse than none. Arms 1, 1b, 1c and 1d live here permanently — 1c +// and 1d carry the four measured evasions as synthetic sources, which is where they belong: a +// permanent test cannot mutate production source. Arms 2 (the cfg-scope revert probe) and 3 +// (planting the same two evasions plus a canonical clone in `gift_delivery.rs`, end to end) are +// executor-run temporary mutations recorded in the change's probe ledger. Arm 4 is the +// `TokenCreated` anchor's own set, kept as a SIBLING test rather than as extra arms of the +// `from`-field resolver test so that the `(4, 10)` pin's own test stays untouched by token work. +// +// Arms 1d and 4(iv) pin the SAME shared function ([`literal_body`]) from both anchors on purpose: +// the truncation defect they cover was found through the token anchor and reached the `from`-field +// one identically, and a shared instrument with a one-sided control is how that stayed invisible. + +/// Build a synthetic source carrying `body` once at production scope and once inside a +/// `#[cfg(test)] pub(crate) mod tests {`. +fn at_both_scopes(body: &str) -> String { + let indented: String = body + .lines() + .map(|line| format!(" {line}\n")) + .collect::(); + format!("fn production() {{\n{indented}}}\n\n#[cfg(test)]\npub(crate) mod tests {{\n fn scoped() {{\n{indented} }}\n}}\n") +} + +/// The four single-line-header forms, parameterised by the origin zone so the same generator +/// produces both the `from: None` population and its `from: Some(Zone::Hand)` negative control. +fn forms_1_to_4(from: &str) -> String { + let needle = anchor(); + [ + format!("events.push({needle}\n object_id: id,\n {from}\n to: Zone::Battlefield,\n record: Box::new(rec),\n}});"), + format!("let zc = {needle}\n object_id: id,\n {from}\n to: Zone::Battlefield,\n record: Box::new(rec),\n}};"), + format!("match event {{ {needle} object_id, {from} to, .. }} => handle(object_id, to), _ => {{}} }}"), + format!("return {needle}\n object_id: id,\n {from}\n to: Zone::Battlefield,\n record: Box::new(rec),\n}};"), + ] + .join("\n") +} + +/// Form 5: the needle inside a `#[cfg(test)]` item whose `fn` signature spans MULTIPLE lines. +/// This is ceiling 1 made executable — the shipped classifier cannot scope it. +fn form_5(from: &str) -> String { + let needle = anchor(); + format!( + "fn production() {{\n events.push({needle} object_id: id, {from} to: Zone::Battlefield, record: r }});\n}}\n\n#[cfg(test)]\npub(crate) fn helper(\n state: &GameState,\n) -> T {{\n events.push({needle} object_id: id, {from} to: Zone::Battlefield, record: r }});\n}}\n" + ) +} + +/// EVASION 1, verbatim: bind the origin zone OUTSIDE the literal and write the field in shorthand. +/// The constructed event is byte-for-byte the same one the authority emits, and the substring +/// `from: None` never appears. Round 3's detector scored this `(0, 0)`. +fn form_6_shorthand_evasion() -> String { + let needle = anchor(); + format!( + "let from = None;\nevents.push({needle}\n object_id: id,\n from,\n to: Zone::Battlefield,\n record: Box::new(rec),\n}});" + ) +} + +/// EVASION 2, verbatim: write `record:` FIRST as a multi-line expression, so `from: None` lands on +/// the literal's ninth line — outside any fixed-size window, and past a NESTED brace pair that a +/// naive scan would stop on. Round 3's detector (`WINDOW = 6`) scored this `(0, 0)`. +fn form_7_window_evasion() -> String { + let needle = anchor(); + format!( + "events.push({needle}\n object_id: id,\n record: Box::new(ZoneChangeRecord {{\n name: name.clone(),\n controller,\n power,\n toughness,\n core_types,\n }}),\n from: None,\n to: Zone::Battlefield,\n}});" + ) +} + +/// EVASION 3, parameterised by the COMMENT the literal carries so the mutant and its control are +/// the SAME construction differing only in that one line. +/// +/// A `//` comment inside the literal whose prose carries an unbalanced `}` — ordinary in this +/// codebase's comment style — decremented the raw brace scan to zero and TRUNCATED the captured +/// body, so every field written after the comment went unseen. Fail-OPEN, and in the direction +/// [`literal_body`]'s own doc used to argue was impossible. +fn form_8_comment_in_literal(comment: &str) -> String { + let needle = anchor(); + format!( + "events.push({needle}\n object_id: id,\n {comment}\n record: Box::new(rec),\n from: None,\n to: Zone::Battlefield,\n}});" + ) +} + +/// EVASION 4: the same truncation reached through a STRING literal rather than a comment. The +/// `}` is inside `"…"`, so it is not the literal's closing brace, but a scan that does not know +/// about string literals cannot tell. +fn form_9_string_brace_evasion() -> String { + let needle = anchor(); + format!( + "events.push({needle}\n object_id: id,\n record: Box::new(rec.named(\"a }} b\")),\n from: None,\n to: Zone::Battlefield,\n}});" + ) +} + +/// EVASION 6, parameterised by the literal's PREFIX so the mutant and its control are the SAME raw +/// string differing only in the leading `b` / `c`. +/// +/// The raw string's content carries a `"` BEFORE its `}`, which is what separates the two scans: a +/// scan that honours `#` delimiters skips the whole literal, while one that falls through to +/// [`skip_escaped`] closes at that embedded `"` and then reads the following `}` as the struct +/// literal's own closing brace. All three prefixes lex as valid Rust +/// (`rustc --edition 2021 --crate-type=lib`, rc=0). +fn form_10_raw_string_brace_evasion(prefix: &str) -> String { + let needle = anchor(); + format!( + "events.push({needle}\n object_id: id,\n record: Box::new(rec.tagged({prefix}#\"a \" }} b\"#)),\n from: None,\n to: Zone::Battlefield,\n}});" + ) +} + +/// The NEGATIVE control for the shorthand branch: a destructuring PATTERN that binds `from` bare is +/// a CONSUMER and must not be counted, or the census would swallow ~20 `engine/src` match sites and +/// stop being a pin on writers. +fn pattern_binding_from() -> String { + let needle = anchor(); + format!("match event {{ {needle} object_id, from, to, record }} => handle(from), _ => {{}} }}") +} + +fn score(src: &str) -> (usize, usize) { + let hits = classify(src, "synthetic"); + ( + hits.iter().filter(|hit| !hit.in_test).count(), + hits.iter().filter(|hit| hit.in_test).count(), + ) +} + +/// The emitter's own shape, VERBATIM: a construction written entirely in field-init shorthand. +/// +/// This is the arm that justifies a second predicate at all. `is_construction`'s `record:` +/// discriminator — the one the `ZoneChanged` anchor uses — scores this `(0, 0)`, so reusing it +/// would have made the token anchor blind to the single site it exists to pin. +fn token_form_shorthand_construction() -> String { + let needle = token_anchor(); + format!("events.push({needle}\n object_id,\n name,\n source_id,\n}});") +} + +/// The `stack.rs` probe's shape: the same three fields with explicit values, and a COMMENT inside +/// the literal (comments there must not knock a field off the completeness check). +fn token_form_explicit_construction() -> String { + let needle = token_anchor(); + format!( + "let tc = {needle}\n object_id: PROBE_ID,\n name: spec.display_name.clone(),\n // the creating source is irrelevant here\n source_id: PROBE_ID,\n}};" + ) +} + +/// The emitter's shorthand shape carrying an ordinary prose comment whose brace is UNBALANCED. +/// +/// This is arm 4(ii)'s claim ("an interleaved comment line must not hide a field") pushed to the +/// case that actually broke it: arm 4(ii)'s comment has no braces at all, so it never exercised +/// the brace scan. +fn token_form_comment_brace_construction() -> String { + let needle = token_anchor(); + format!( + "events.push({needle}\n object_id,\n // set by the copy tail, see the match arm ending in }}\n name,\n source_id,\n}});" + ) +} + +/// The NEGATIVE control: the four consumer PATTERN shapes that actually occur in `engine/src`. +/// +/// `trigger_matchers.rs`, `log.rs`, `destroy.rs`, `engine_debug.rs` and a dozen others are full of +/// these — 26 of the anchor's 38 occurrences are patterns. Counting them would turn the pinned +/// multiset into churn that moves on every unrelated consumer edit. +fn token_patterns() -> String { + let needle = token_anchor(); + [ + format!("match event {{ {needle} .. }} => LogCategory::Token, _ => other }}"), + format!("match event {{ {needle} object_id, .. }} => Some(*object_id), _ => None }}"), + format!("let {needle} object_id, source_id, .. }} = event else {{ return None }};"), + format!("events.iter().any(|e| matches!(e, {needle} name, .. }} if name == \"Soldier\"))"), + ] + .join("\n") +} + +fn token_score(src: &str) -> (usize, usize) { + let hits = classify_tokens(src, "synthetic"); + ( + hits.iter().filter(|hit| !hit.in_test).count(), + hits.iter().filter(|hit| hit.in_test).count(), + ) +} + +/// ARM 4 — the `TokenCreated` anchor's anti-vacuity set. +/// +/// The `(2, 10)` pin above is only a measurement if the same instrument resolves differently on +/// different inputs. These three arms prove it can score non-zero, that it separates production +/// from `#[cfg(test)]` scope, and that it refuses consumers. +#[test] +fn the_token_created_resolver_keys_on_cfg_scope_and_on_field_completeness() { + // (i) POSITIVE CONTROL + SCOPE DISCRIMINATION, on the emitter's real shorthand shape. + assert_eq!( + token_score(&at_both_scopes(&token_form_shorthand_construction())), + (1, 1), + "arm 4(i): the emitter writes all three fields in SHORTHAND. It must be seen once at \ + production scope and once inside a `#[cfg(test)] pub(crate) mod tests {{`. The \ + `ZoneChanged` anchor's `record:`-keyed `is_construction` scores this (0, 0), which is \ + exactly why `is_token_construction` keys on field completeness instead" + ); + + // (ii) The explicit-value shape, with a comment inside the literal. + assert_eq!( + token_score(&at_both_scopes(&token_form_explicit_construction())), + (1, 1), + "arm 4(ii): an explicit-value construction (the `stack.rs` probe's shape) must score the \ + same as the shorthand one, and an interleaved comment line must not hide a field from the \ + completeness check" + ); + + // (iii) NEGATIVE CONTROL: the consumer patterns. This is the arm that keeps the pin from + // swallowing the 26 match sites and becoming churn. + assert_eq!( + token_score(&at_both_scopes(&token_patterns())), + (0, 0), + "arm 4(iii): a destructuring PATTERN elides at least one field with `..`, so \ + `is_token_construction` rejects it. Counting these would add 26 consumer sites across 15 \ + files to the multiset" + ); + + // (iv) The same shape with an UNBALANCED brace in that comment. This is (ii)'s claim taken to + // the case that broke it: (ii)'s comment is brace-free, so it never reached the brace + // scan. Both anchors share `literal_body`, so this pins it from the token side while + // arm 1d pins it from the `ZoneChanged` side. + assert_eq!( + token_score(&at_both_scopes(&token_form_comment_brace_construction())), + (1, 1), + "arm 4(iv): a `}}` inside a `//` comment is prose, not the literal's closing brace. A raw \ + brace scan truncates the body there, so `name` and `source_id` are never named and \ + `is_token_construction` returns false — the emitter itself would go uncounted, which is \ + fail-OPEN" + ); +} + +/// The exact shape of the round 10 defect: a single-id publish written NEXT TO the guarded one. +fn publish_form(receiver_and_field: &str) -> String { + format!("{receiver_and_field}.push(object_id);") +} + +/// The MULTI-LINE field access rustfmt actually produces (`token_copy.rs:321-323`), which is what +/// makes a line-oriented rule under-count this surface. +fn publish_form_multiline() -> String { + "pending\n .created_ids\n .push(object_id);".to_string() +} + +/// The non-publish tails that really occur in `engine/src`, one of each shape from the measured +/// tail vocabulary: bulk assign, bulk extend, clear, clone, len, index, contains. +fn non_publish_forms() -> String { + [ + "state.last_created_token_ids = created_ids;", + "state.last_created_token_ids.extend(status.created_ids);", + "state.last_created_token_ids.clear();", + "let v = state.last_created_token_ids.clone();", + "let n = state.last_created_token_ids.len();", + "let first = state.last_created_token_ids[0];", + "let has = state.last_created_token_ids.contains(&object_id);", + "pending.created_ids = created_ids;", + "pending\n .created_ids\n .extend(state.last_created_token_ids.clone());", + ] + .join("\n") +} + +fn publish_score(src: &str) -> (usize, usize) { + let hits = classify_publishes(src, "synthetic"); + ( + hits.iter().filter(|hit| !hit.in_test).count(), + hits.iter().filter(|hit| hit.in_test).count(), + ) +} + +/// Conjunct 4's scorer. Same walker, other half of the mutator partition. +fn ambiguous_score(src: &str) -> (usize, usize) { + let hits = classify_ambiguous_mutators(src, "synthetic"); + ( + hits.iter().filter(|hit| !hit.in_test).count(), + hits.iter().filter(|hit| hit.in_test).count(), + ) +} + +/// ARM 5 — the third anchor's anti-vacuity set. +/// +/// The `(2, 0)` and `(5, 0)` pins are only measurements if the same instrument resolves +/// differently on different inputs. These arms prove it scores non-zero on the defect's own shape, +/// separates production from `#[cfg(test)]` scope, sees the multi-line field access a line-oriented +/// rule cannot, refuses every bulk/read form, refuses prose in all three positions it is skipped +/// in, fails CLOSED on a tail it cannot read, and PARTITIONS the mutator vocabulary between the two +/// classifiers — including the assert that `.extend(std::iter::once(id))` is invisible to the +/// single-id one, which is the limit this set exists to state rather than to hide. +#[test] +fn the_anaphora_publish_resolver_keys_on_cfg_scope_and_on_the_call_tail() { + // (i) POSITIVE CONTROL + SCOPE DISCRIMINATION, on ledger 3 — the round 9 defect's own shape. + assert_eq!( + publish_score(&at_both_scopes(&publish_form("state.last_created_token_ids"))), + (1, 1), + "arm 5(i): a bare `state.last_created_token_ids.push(id)` is the UNGUARDED ledger-3 publish \ + that shipped at four sites before round 9. It must be seen once at production scope and \ + once inside a `#[cfg(test)] pub(crate) mod tests {{`" + ); + + // (ii) The BUFFER, in the multi-line shape rustfmt writes. This is the `-U` lesson made + // executable: a line-oriented rule reports 9 buffer writers where there are 11. + assert_eq!( + publish_score(&at_both_scopes(&publish_form_multiline())), + (1, 1), + "arm 5(ii): `pending` / `.created_ids` / `.push(…)` split across three lines is the round \ + 10 defect's own shape AND the shape a line-oriented query cannot express. If this scores \ + (0, 0) the anchor has silently become line-oriented and the class it pins is unpinned" + ); + + // (iii) NEGATIVE CONTROL for the SINGLE-ID classifier, and note carefully what it does NOT + // claim. A bulk assign, `.clear`, `.clone`, `.len`, indexing and `.contains` cannot + // introduce an id — that is a property of the operation. `.extend` CAN + // (`.extend(std::iter::once(id))`); it is absent from this score only because its + // argument is unreadable, and it is accounted for by conjunct 4 / arm 5(vii), not + // dismissed here. Counting the read forms would make the pin churn on the ~150 reads + // and bulk republishes that are deliberately out of scope. + assert_eq!( + publish_score(&at_both_scopes(&non_publish_forms())), + (0, 0), + "arm 5(iii): the single-id classifier must not fire on a bulk assign, a `.clear`, or a \ + read (`.clone`, `.len`, indexing, `.contains`) — none of those can introduce an id, and \ + counting them would put every consumer file in the multiset. The `.extend` lines in this \ + fixture carry BULK arguments; they are uncounted here because this classifier cannot read \ + arguments at all, which is exactly what conjunct 4 and arm 5(vii) exist to bound" + ); + + // (iv) NEGATIVE CONTROL: prose, in the FULL-LINE `//` shape. The doc comments this very change + // added mention `pending.created_ids.push(id)` verbatim; a comment-blind anchor fires on + // them. Arm 5(viii) covers the other two shapes the same prose can take. + assert_eq!( + publish_score(&at_both_scopes( + "// the guard shipped with an UNGUARDED `pending.created_ids.push(id)` below it" + )), + (0, 0), + "arm 5(iv): a full-line `//` comment publishes nothing. This change's own `counters.rs` \ + and `token.rs` docs quote the defect verbatim, so a comment-blind anchor would pin its \ + own prose" + ); + + // (v) `.insert(` is the other single-element `Vec` write, and a fix that only closed `.push` + // would leave it open with no arm saying so. + assert_eq!( + publish_score(&at_both_scopes( + "state.last_created_token_ids.insert(0, object_id);" + )), + (1, 1), + "arm 5(v): `.insert(0, id)` publishes exactly one id, same as `.push(id)`" + ); + + // (vi) FAIL-CLOSED on an unreadable tail. `call_tail` skips `//` comments but not block + // comments, so a `/* … */` between the field and its call would hide the call. It is + // counted instead, which ADDS a hit and fails the exact multiset. + assert_eq!( + publish_score(&at_both_scopes( + "state.last_created_token_ids /* see below */ .extend(v);" + )), + (1, 1), + "arm 5(vi): a block comment between the field and its method call is a tail `call_tail` \ + cannot read. It is counted, so it ADDS an unexpected hit and FAILS the exact multiset — \ + the same fail-CLOSED choice `FN_PREFIX_ALLOW_SET`'s abort makes for an unknown `fn` prefix" + ); + + // (vii) THE ARGUMENT-AMBIGUOUS CLASS, both halves. `.extend(std::iter::once(id))` publishes + // exactly ONE id — this is the measured limit of the single-id classifier, asserted + // here rather than left as a claim in prose — and conjunct 4's classifier is what keeps + // it accounted for. The third assert is the one that matters: the two classifiers must + // PARTITION the vocabulary, so a form counted by one is never counted by the other and + // neither pin can absorb the other's churn. + let single_id_extend = "state.last_created_token_ids.extend(std::iter::once(object_id));"; + assert_eq!( + publish_score(&at_both_scopes(single_id_extend)), + (0, 0), + "arm 5(vii): `.extend(std::iter::once(id))` publishes one id and is INVISIBLE to the \ + single-id classifier, because no text scanner can separate it from `.extend(other_vec)`. \ + If this ever scores non-zero the classifier has started guessing at arguments, and the \ + guess is what will be wrong" + ); + assert_eq!( + ambiguous_score(&at_both_scopes(single_id_extend)), + (1, 1), + "arm 5(vii): the same call MUST be seen by the ambiguous-mutator classifier, once per \ + scope. If this scores (0, 0) the fail-OPEN of arm 5(vii)'s first assert is unbounded \ + again and the class this anchor exists for is only half pinned" + ); + assert_eq!( + ambiguous_score(&at_both_scopes(&publish_form( + "state.last_created_token_ids" + ))), + (0, 0), + "arm 5(vii): `.push(id)` belongs to conjunct 1, not conjunct 4. The two classifiers \ + partition the twelve-verb mutator vocabulary; an overlap would double-count a publish and \ + make each pin churn on the other's edits" + ); + + // (viii) COMMENT POSITION, the two shapes arm 5(iv)'s full-line rule does not reach — and, + // just as importantly, the two controls proving `code_span` cannot eat CODE. The + // removals are the only place this anchor deletes text before scanning, so a fail-OPEN + // introduced there would be invisible to every other arm. + assert_eq!( + publish_score(&at_both_scopes( + "let n = 1; // shipped with state.last_created_token_ids.push(id) below" + )), + (0, 0), + "arm 5(viii): a TRAILING `//` comment publishes nothing. Re-wrapping one of this change's \ + own doc lines into this position is the most likely way to fail the census with no defect \ + present" + ); + assert_eq!( + publish_score(&at_both_scopes( + "/* shipped with pending.created_ids.push(id) below */" + )), + (0, 0), + "arm 5(viii): a full-line BLOCK comment publishes nothing either" + ); + assert_eq!( + publish_score(&at_both_scopes( + "/*count=*/ state.last_created_token_ids.push(object_id);" + )), + (1, 1), + "arm 5(viii): CONTROL — the leading-`/*` removal must stop at the first `*/`. \ + `engine.rs` writes `/*tapped=*/` argument labels in exactly this shape, and a removal \ + that ran to end-of-line would silently unsee any publish written after one" + ); + assert_eq!( + publish_score(&at_both_scopes( + "let u = \"http://x\"; state.last_created_token_ids.push(object_id);" + )), + (1, 1), + "arm 5(viii): CONTROL — the trailing-`//` removal is suppressed when a `\"` precedes the \ + slashes, because that `//` is inside a string. Without this guard a URL on the same line \ + would hide a real publish, which is fail-OPEN" + ); +} + +#[test] +fn the_census_resolver_keys_on_cfg_scope_and_on_the_literal_from_field() { + // ── ARM 1 — POSITIVE CONTROL / keying. ────────────────────────────────────────────────── + // + // SOURCE A: forms 1–4, each once at production scope and once inside a `#[cfg(test)]` module. + let source_a = at_both_scopes(&forms_1_to_4("from: None,")); + assert_eq!( + score(&source_a), + (4, 4), + "arm 1 / source A: four single-line-header construction forms — a bare `events.push`, a \ + `let` binding, a match PATTERN, and a `return` — must each be seen once at production \ + scope and once inside a `#[cfg(test)] pub(crate) mod tests {{`. A bare anchor sees all \ + four shapes; the qualifier-anchored detector this replaced saw only two." + ); + + // SOURCE B: form 5 alone. BOTH copies score PRODUCTION — that is ceiling 1, MEASURED here + // rather than asserted in prose. The cfg-attributed copy is not scoped because its `fn` header + // line ends in `(`, which satisfies neither `opens_module` nor `ends_with('{')`. + let source_b = form_5("from: None,"); + assert_eq!( + score(&source_b), + (2, 0), + "arm 1 / source B: a `#[cfg(test)]` item with a MULTI-LINE `fn` signature is a known, \ + measured ceiling of `cfg_test_scoped_lines` — BOTH copies (the correctly-scoped \ + production one and the cfg-attributed one the classifier cannot see) score production. \ + This is asserted because it is measured, not because it is desirable; it is fail-CLOSED \ + for this census because the per-file multiset above rejects unexpected files." + ); + + // NEGATIVE CONTROL on both sources: the window keys on `from: None`, not on the anchor alone. + // One instrument resolving (4,4), (2,0) and (0,0) on different inputs is a measurement rather + // than a constant. + assert_eq!( + score(&at_both_scopes(&forms_1_to_4("from: Some(Zone::Hand),"))), + (0, 0), + "arm 1 negative control (source A): a `Some(from)` construction is the `move_to_zone` \ + sibling and must NOT be counted" + ); + assert_eq!( + score(&form_5("from: Some(Zone::Hand),")), + (0, 0), + "arm 1 negative control (source B): same, for the multi-line-header form" + ); + + // ── ARM 1c — THE TWO MEASURED EVASIONS, permanently pinned. ───────────────────────────── + // + // Both COMPILE and both construct the identical event. Round 3's window+substring detector + // scored each of these `(0, 0)`; the whole point of the brace scan and the field + // classification is that they now score like any other construction. + assert_eq!( + score(&at_both_scopes(&form_6_shorthand_evasion())), + (1, 1), + "arm 1c / evasion 1 (field-init shorthand): `let from = None;` + `from,` is the same \ + construction spelled differently. A detector that matches the string `from: None` scores \ + it (0, 0) and lets the clone through" + ); + assert_eq!( + score(&at_both_scopes(&form_7_window_evasion())), + (1, 1), + "arm 1c / evasion 2 (record-first): a multi-line `record:` initializer pushes `from: None` \ + onto the literal's ninth line, past a NESTED brace pair. A fixed six-line window scores \ + it (0, 0); scanning to the literal's own closing brace does not" + ); + + // ── ARM 1d — THE TRUNCATION EVASIONS, two-sided on the SAME literal. ──────────────────── + // + // Round 8's scan counted every `}` as code, so a `}` inside a comment or a string closed the + // literal EARLY and the fields written after it were never captured. That is the fail-OPEN + // direction — it REMOVES a hit — which is why it needs a permanent arm rather than the + // "can only ADD a hit" reasoning the other ceilings rest on. + // + // The pair below is the discriminating control: both sources are the same construction, and + // they differ only in whether the comment's braces are balanced. Under the raw scan the + // balanced one scored (1, 1) and the stray-brace one scored (0, 0). + assert_eq!( + score(&at_both_scopes(&form_8_comment_in_literal( + "// the Some(rec) arm above already closed with }" + ))), + (1, 1), + "arm 1d / evasion 3 (stray `}}` in a comment): a comment writes no field and its braces \ + are not code. A raw brace scan treats that `}}` as the literal's own closing brace, \ + returns a TRUNCATED body, and never sees the `from: None` written after it — scoring \ + (0, 0) on a construction that compiles and emits the event" + ); + assert_eq!( + score(&at_both_scopes(&form_8_comment_in_literal( + "// the Some(rec) arm above already closed with { }" + ))), + (1, 1), + "arm 1d control: the SAME construction whose comment braces BALANCE. It scores (1, 1) \ + under the raw scan too, which is what makes the arm above a measurement of the stray \ + brace rather than of comments in general" + ); + assert_eq!( + score(&at_both_scopes(&form_9_string_brace_evasion())), + (1, 1), + "arm 1d / evasion 4 (`}}` inside a string literal): same truncation, reached through a \ + string rather than a comment. AT THIS TIP `engine/src` carries 51 `'{{'`/`'}}'` char \ + literals on 46 distinct lines (`grep -raoP \"'[{{}}]'\" crates/engine/src | wc -l`) and \ + 156 raw-string openers (`grep -raoP '(? u8 {\n 2\n}\n"; + let message = top_level_fn_headers(foreign) + .expect_err("arm 1b(ii): an unrecognised `fn` prefix must fail the census, not be skipped"); + assert!( + message.contains("EXTEND") && message.contains("const "), + "arm 1b(ii): the abort must name the offending prefix and tell the reader to extend the \ + allow-set; got {message:?}" + ); + + // (iii) …and the prefix the remedy names must actually BE in the allow-set, or the remedy is a + // contradiction. This is the assertion that fails if `pub(in crate::game) ` is removed. + let local = "pub(crate) fn authority() {\n let _ = 1;\n}\n\npub(in crate::game) fn helper() {\n let _ = 2;\n}\n"; + let headers = top_level_fn_headers(local).expect( + "arm 1b(iii): `pub(in crate::game) ` is a real engine idiom and must be recognised", + ); + assert_eq!( + enclosing_fn(&headers, 6), + Some("helper"), + "arm 1b(iii): `pub(in crate::game) fn` is the one column-0 prefix in `engine/src` outside \ + the original four (24 instances). Aborting on it would make an unrelated edit red, and \ + the abort's advised remedy — extend the allow-set — is only followable if the set can \ + actually hold it" + ); + + // Positive control on the same instrument: a zero/abort result is only meaningful if the + // instrument can also succeed on the real file. + let zones = engine_src("game/zones.rs"); + assert!( + top_level_fn_headers(&zones).is_ok(), + "arm 1b positive control: the real zones.rs must parse cleanly under the same allow-set" + ); +} diff --git a/crates/engine/tests/integration/loop_shortcut_offer_writer_census.rs b/crates/engine/tests/integration/loop_shortcut_offer_writer_census.rs index 8ea900fdf7..f2255f7c8e 100644 --- a/crates/engine/tests/integration/loop_shortcut_offer_writer_census.rs +++ b/crates/engine/tests/integration/loop_shortcut_offer_writer_census.rs @@ -70,7 +70,7 @@ struct Hit { /// /// The naive "nearest preceding attribute" rule is measured wrong and yields /// false TEST verdicts, so it is deliberately not used. -fn cfg_test_scoped_lines(src: &str) -> Vec { +pub(super) fn cfg_test_scoped_lines(src: &str) -> Vec { let lines: Vec<&str> = src.lines().collect(); let mut scoped = vec![false; lines.len()]; let mut i = 0usize; @@ -137,7 +137,7 @@ fn classify(src: &str, needle: &str, file: &str) -> Vec { .collect() } -fn rs_files(root: &Path) -> Vec { +pub(super) fn rs_files(root: &Path) -> Vec { let mut out = Vec::new(); let mut stack = vec![root.to_path_buf()]; while let Some(dir) = stack.pop() { diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 2fa5487c13..426ac72057 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -42,6 +42,7 @@ mod baleful_mastery_regression; mod banding_combat; mod batched_trigger_subject_count; mod battle_of_wits; +mod battlefield_entry_authority_census; mod bbfu10_entered_this_turn_snapshot; mod bbfu7_attacks_if_able_not_goad; mod belbe_thornbow_life_loss; diff --git a/crates/engine/tests/integration/token_zone_change_index.rs b/crates/engine/tests/integration/token_zone_change_index.rs index b5da8b4f74..0eb6b9d29c 100644 --- a/crates/engine/tests/integration/token_zone_change_index.rs +++ b/crates/engine/tests/integration/token_zone_change_index.rs @@ -10,20 +10,73 @@ //! `(definition_ref, turn_zone_change_index)` — CR 603.2c, "an ability triggers only once each //! time its trigger event occurs" — so a SECOND same-turn token batch collided with the first on //! `(def, 0)` and its fire was silently swallowed. - -use engine::game::effects::{incubate, token}; +//! +//! # REVERT-PROBE convention — read this before trusting an anchor +//! +//! Most assertions here carry an inline `REVERT-PROBE` anchor naming the mutation that should +//! break that assertion. **`RUN` in an anchor tag is an INSTRUCTION to you, not a claim that +//! anyone ran it.** Only anchors that also say `MEASURED` were executed, and those carry the +//! verbatim failure point (file, line, `left`/`right`). Anything else is a PREDICTION. +//! +//! This distinction is load-bearing: two anchors in this file (`PROBE X` and `PROBE Y`, on the +//! gift-token tests) named failure points their recipes never reached, and read as validated for +//! multiple review rounds precisely because nothing separated "stated" from "executed". +//! +//! The failure mode to watch for is **RECIPE SCOPE**. An *authority-wide* revert (neutering +//! `zones::record_and_emit_entry_from_no_zone` itself) also degrades the OTHER producer in the +//! same fixture — usually a priming batch the test relies on — which can trip an upstream +//! reach-guard and kill the test EARLIER than the anchor's named point. A *site-isolated* revert +//! (one emit site, primer left on the real authority) does not. Prefer site-isolated recipes. +//! +//! A discrimination claim needs BOTH mutants, and each must fail THAT assertion, not merely the +//! suite: **DROP** (delete the fix) and **TRIVIALIZE** (keep the shape, make the recorded index a +//! meaningless constant). Scope a trivialize constant to the `from: None -> Battlefield` arm — a +//! global one is dominated by the ordinary-move `TurnRecordIndexMismatch` invariant in `zones.rs` +//! and panics before any assertion here. +//! +//! **Filter trap:** `cargo test --test integration -- --exact` runs ZERO tests and +//! exits `0`. The module path is mandatory: +//! `cargo test -p phase-engine --test integration token_zone_change_index:: -- --exact`. +//! Always check the `N passed` count — a vacuous filter reports `ok. 0 passed`. +//! +//! MEASURED at this tip: `second_same_turn_token_batch_still_triggers`, +//! `mixed_group_sibling_then_token_each_fire_the_batched_trigger`, +//! `a_realized_copy_token_entry_and_a_same_turn_token_batch_take_distinct_indices` (both arms plus +//! a counter-control on its pre-fix form), the inline probe inside +//! `suppressed_liminal_copy_token_entry_is_recorded_once`, and gift-token `PROBE X` / `PROBE Y`. +//! +//! STATED BUT NOT EXECUTED here — treat as predictions until run: +//! `mixed_group_sibling_last_also_fires`, +//! `battlefield_entries_this_turn_counts_each_token_exactly_once`, +//! `conjured_battlefield_entry_after_a_token_batch_fires_the_batched_trigger`, +//! `unpaused_copy_token_entry_is_realized_by_the_copy_target_action_itself`, +//! the three `suppressed_liminal_copy_token_entry_*` realization-pause tests +//! (`..._mandatory_as_enters_choice`, `..._as_enters_choice_with_a_second_pause`, +//! `..._etb_counter_ordering_pause`), the remaining `suppressed_liminal_...` inline anchors, and +//! the `*_after_a_token_batch_fires_the_batched_trigger` family for gift, copy-token tail, +//! modification-paused copy, counter-paused token, counter-paused attached, counter-paused copy, +//! and incubate-resume. + +use engine::game::effects::{conjure, gift_delivery, incubate, token, token_copy}; +use engine::game::filter::{matches_target_filter_on_zone_change_record, FilterContext}; +use engine::game::game_object::AttachTarget; +use engine::game::quantity::resolve_quantity; use engine::game::scenario::{GameRunner, GameScenario, P0}; use engine::game::triggers::{drain_order_triggers_with_identity, process_triggers}; use engine::types::ability::{ - AbilityDefinition, AbilityKind, Effect, PtValue, QuantityExpr, ResolvedAbility, TargetFilter, - TargetRef, TriggerDefinition, + AbilityDefinition, AbilityKind, ChosenAttribute, ConjureCard, ConjureSource, + ContinuousModification, Effect, PtValue, QuantityExpr, QuantityRef, ResolvedAbility, + TargetFilter, TargetRef, TriggerDefinition, }; use engine::types::actions::GameAction; +use engine::types::counter::CounterType; use engine::types::events::GameEvent; use engine::types::game_state::{GameState, WaitingFor}; use engine::types::identifiers::ObjectId; -use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; +use engine::types::keywords::GiftKind; +use engine::types::mana::{ManaColor, ManaCost, ManaCostShard, ManaType, ManaUnit}; use engine::types::phase::Phase; +use engine::types::player::PlayerId; use engine::types::triggers::TriggerMode; use engine::types::zones::Zone; @@ -86,10 +139,11 @@ fn mint_token_batch(state: &mut GameState, source: ObjectId, count: i32) -> Vec< /// /// `incubate.rs` was one of SEVEN battlefield-entry emit sites that built a `ZoneChanged` record /// with `snapshot_for_zone_change` and emitted it without ever reaching the recorder, so it shipped -/// the index-`0` placeholder. It is routed through `record_zone_change` by this change because -/// these very tests drive it; the six that remain (`conjure.rs`, `counters.rs` ×2 — the `:526` -/// inline emit and `push_token_entry_events` — `token_copy.rs` ×2, `gift_delivery.rs`) are the -/// class-wide follow-up. +/// the index-`0` placeholder. It was routed through `record_zone_change` first because these very +/// tests drive it. The class is now CLOSED: all six are now routed through +/// `zones::record_and_emit_entry_from_no_zone`, the single `from: None → Battlefield` record+emit +/// authority, enforced structurally by `battlefield_entry_authority_census.rs`. Each has its own +/// discriminator and empty-ledger control at the bottom of this file. fn incubate_batch(state: &mut GameState, source: ObjectId, count: i32) -> Vec { let ability = ResolvedAbility::new( Effect::Incubate { @@ -138,10 +192,26 @@ fn token_ids(state: &GameState) -> Vec { /// `ChangesZone` trigger, must fire the trigger TWICE — once per batch — because each batch is a /// distinct trigger event. /// -/// REVERT-PROBE (discriminating, RUN): restore the direct -/// `snapshot_for_zone_change` emit in `push_committed_token_entry_events` (index left at the `0` +/// REVERT-PROBE (discriminating, RUN): replace the +/// `zones::record_and_emit_entry_from_no_zone` call inside `push_committed_token_entry_events` +/// with a bare `snapshot_for_zone_change` + `events.push(ZoneChanged)` (index left at the `0` /// placeholder) ⇒ both batches key on `(def, 0)`, the second is dropped by /// `batched_zone_change_already_collected`, and P0 gains 1 life instead of 2. +/// +/// MEASURED (both arms, this tip). DROP, as written above: fails at the life assertion below — +/// `left: 1 right: 2`, the first failure, no earlier guard trips. TRIVIALIZE, keeping +/// `record_zone_change` wired so both per-turn ledgers hold the REAL index and forcing only the +/// EMITTED record's index to `0`: same assertion, same `left: 1 right: 2`. The trivialize arm +/// proves the stronger property neither prediction claimed — this test discriminates the +/// **emitted** `turn_zone_change_index` specifically, not ledger presence. +/// +/// RECIPE-SCOPE NOTE: this recipe is authority-wide, so it degrades the PRIMING batch too. The +/// test survives that only by arithmetic coincidence — batch 1's reach-guard below expects delta +/// `1`, and a degraded batch 1 still fires exactly once (both its tokens key on `(def, 0)`; one +/// batch = one fire either way), so the guard is INSENSITIVE to the mutation and control reaches +/// the named assertion. If that guard's expected value ever changes, re-measure this probe before +/// trusting it: the same authority-wide shape made two sibling anchors in this file name failure +/// points their recipes never reached. #[test] fn second_same_turn_token_batch_still_triggers() { let mut scenario = GameScenario::new(); @@ -224,9 +294,19 @@ fn second_same_turn_token_batch_still_triggers() { /// Both mechanisms are routed now (`token.rs` and `incubate.rs`), so this passes in either order; /// `mixed_group_sibling_last_also_fires` is the reversed-order twin. /// -/// REVERT-PROBE (discriminating, RUN): restore the direct `snapshot_for_zone_change` emit in -/// `push_committed_token_entry_events` ⇒ the token batch ships index `0`, collides with the -/// Incubator's `0`, and P0 gains 1 life instead of 2. +/// REVERT-PROBE (discriminating, RUN): replace the `zones::record_and_emit_entry_from_no_zone` +/// call inside `push_committed_token_entry_events` with a bare `snapshot_for_zone_change` + +/// `events.push(ZoneChanged)` ⇒ the token batch ships index `0`, collides with the Incubator's +/// `0`, and P0 gains 1 life instead of 2. +/// +/// MEASURED (both arms, this tip). DROP: fails at the life assertion below — `left: 1 right: 2`, +/// first failure, no earlier guard trips. TRIVIALIZE (ledgers keep the real index, only the +/// EMITTED index forced to `0`): same assertion, same values. +/// +/// RECIPE-SCOPE NOTE: unlike its same-recipe sibling above, this test is STRUCTURALLY immune to +/// the priming-producer hazard — its primer is Incubate, which reaches the authority through +/// `incubate.rs`, NOT through `push_committed_token_entry_events`. The recipe degrades only the +/// token producer, so the sibling's real index and its reach-guard below are untouched. #[test] fn mixed_group_sibling_then_token_each_fire_the_batched_trigger() { let mut scenario = GameScenario::new(); @@ -352,14 +432,25 @@ fn mixed_group_sibling_last_also_fires() { ); } -/// MUST-NOT-FLIP for the paired deletion: routing token entries through `record_zone_change` -/// (which performs the CR 403.3 battlefield-entry bookkeeping itself) means the emit sites must -/// NOT also call `record_battlefield_entry`. +/// Routing token entries through `record_zone_change` (which performs the CR 608.2i +/// battlefield-entry bookkeeping itself) means an emit site must NOT also call +/// `record_battlefield_entry`. +/// +/// NOT the paired-deletion must-not-flip. This drive's route +/// (`token::apply_create_token_after_replacement_with_created_ids`) never had such a call to +/// delete: measured on `4b34e5465`, the seven pre-change `record_battlefield_entry` call sites in +/// `engine/src` outside `restrictions.rs` are `conjure.rs:191`, `counters.rs:518/558/637`, +/// `gift_delivery.rs:157` and `token_copy.rs:851/969` — none in `token.rs`, whose emitter already +/// relied on `record_zone_change` doing the bookkeeping. The paired-deletion claim belongs to +/// `assert_site_records`, which drives the sites the deletion actually touched, and is stated +/// there. /// -/// REVERT-PROBE (discriminating, RUN): re-add the deleted +/// REVERT-PROBE (discriminating, RUN): ADD a /// `crate::game::restrictions::record_battlefield_entry` call in /// `apply_create_token_after_replacement_with_created_ids` ⇒ every token appears TWICE in -/// `battlefield_entries_this_turn` and the per-id count assertion fails with 2. +/// `battlefield_entries_this_turn` and the per-id count assertion fails with 2. That is a +/// forward-direction discriminator — it proves this assertion can fail — not evidence that the +/// call was ever there. #[test] fn battlefield_entries_this_turn_counts_each_token_exactly_once() { let mut scenario = GameScenario::new(); @@ -387,7 +478,8 @@ fn battlefield_entries_this_turn_counts_each_token_exactly_once() { assert_eq!( entries, 1, "token {id:?} is recorded in battlefield_entries_this_turn exactly once \ - (re-adding the deleted record_battlefield_entry ⇒ 2)" + (ADDING a record_battlefield_entry call at this route ⇒ 2; this route never had \ + one to delete — see the doc comment)" ); } @@ -407,7 +499,7 @@ fn battlefield_entries_this_turn_counts_each_token_exactly_once() { } } -// ───────── the SUPPRESS route (CR 403.3 + CR 603.6a) ───────── +// ───────── the SUPPRESS route (CR 608.2i + CR 603.6a) ───────── // // `finalize_committed_liminal_token_entry_from_action` records AND emits inline only on the // `TokenEntryEventEmission::Emit` route. On `Suppress` — reached solely from the liminal branch of @@ -453,7 +545,7 @@ fn battlefield_entries_this_turn_counts_each_token_exactly_once() { /// which is the only production route to `TokenEntryEventEmission::Suppress`. const VIZIER_ORACLE: &str = "You may have this creature enter as a copy of any creature on the battlefield, except if this creature was embalmed, the token has no mana cost, it's white, and it's a Zombie in addition to its other types.\nEmbalm {3}{U}{U}"; -/// CR 403.3 + CR 603.6a: a liminal copy-token entry committed on the `Suppress` route must land on +/// CR 608.2i + CR 603.6a: a liminal copy-token entry committed on the `Suppress` route must land on /// both per-turn ledgers exactly once, describing the REALIZED copy, and must emit its entry pair /// exactly once — all of it from the single realization the flush performs, never half of it. /// @@ -464,10 +556,14 @@ const VIZIER_ORACLE: &str = "You may have this creature enter as a copy of any c /// point (a), inside `engine_replacement::finish_copy_target_choice_entry`. /// /// REVERT-PROBE (discriminating, RUN): replace the `Suppress` park in -/// `token::finalize_committed_liminal_token_entry_from_action` with the pre-lifecycle -/// `record_committed_token_entry(state, object_id);` ⇒ the row is written from the pre-copy +/// `token::finalize_committed_liminal_token_entry_from_action` with a pre-lifecycle RECORD-ONLY +/// inline — take `state.objects.get(&object_id)`'s +/// `snapshot_for_zone_change(object_id, None, Zone::Battlefield)` and pass it straight to +/// `restrictions::record_zone_change`, emitting nothing — ⇒ the row is written from the pre-copy /// Shapeshifter (assertion (2b) reads `name: "Vizier of Many Faces"`, `power: Some(0)`) and nothing -/// is ever parked for the flush to realize, so assertion (3)'s emit count is 0. +/// is ever parked for the flush to realize, so assertion (3)'s emit count is 0. The substitution +/// must be record-only (NOT `zones::record_and_emit_entry_from_no_zone`, which also emits), or +/// assertion (3) would see the emit and the isolation claim would be lost. #[test] fn suppressed_liminal_copy_token_entry_is_recorded_once() { let mut scenario = GameScenario::new(); @@ -577,7 +673,7 @@ fn suppressed_liminal_copy_token_entry_is_recorded_once() { }); runner.advance_until_stack_empty(); - // (1) DISCRIMINATOR: the suppressed-emission entry is recorded exactly once (CR 403.3). + // (1) DISCRIMINATOR: the suppressed-emission entry is recorded exactly once (CR 608.2i). assert_eq!( runner .state() @@ -642,10 +738,15 @@ fn suppressed_liminal_copy_token_entry_is_recorded_once() { // cannot disagree by construction — that structural agreement is what this pins. // // REVERT-PROBE (discriminating, RUN): delete the `record_zone_change` call inside - // `token::record_committed_token_entry` and push the row onto `zone_changes_this_turn` - // directly ⇒ `battlefield_entries_this_turn` never gets its row and the - // `.expect("...has a battlefield-entry row")` below panics, while (2b) above stays - // green — isolating the flip to the SECOND ledger. + // `zones::record_and_emit_entry_from_no_zone` and push the row onto + // `zone_changes_this_turn` directly, leaving the `0` placeholder ⇒ + // `battlefield_entries_this_turn` never gets its row. MEASURED failure point: + // assertion (1) above, at this file's `battlefield_entries_this_turn` count + // (`left: 0, right: 1`) — that earlier count assertion dominates, so the test dies + // there. The `.expect("...has a battlefield-entry row")` below would panic for the + // same missing row but is never reached, and (2b) is never evaluated. This probe + // therefore pins the SECOND ledger losing its row; it does NOT isolate one assertion + // against another. let battlefield_row = runner .state() .battlefield_entries_this_turn @@ -655,7 +756,7 @@ fn suppressed_liminal_copy_token_entry_is_recorded_once() { .clone(); assert_eq!( battlefield_row.name, entry_row.name, - "both CR 403.3 ledgers describe the same entry, so they must name the same creature" + "both CR 608.2i ledgers describe the same entry, so they must name the same creature" ); // The measured subtypes here are ["Zombie"], and that is the FIXTURE, not a copy rule: // `GameScenario::add_creature` (game/scenario.rs:357) sets only `CoreType::Creature` and @@ -670,11 +771,11 @@ fn suppressed_liminal_copy_token_entry_is_recorded_once() { // from `battlefield_entry_matches_filter` and a different one from a zone-change scan. assert_eq!( battlefield_row.subtypes, entry_row.subtypes, - "both CR 403.3 ledgers snapshot the same object, so their subtypes agree" + "both CR 608.2i ledgers snapshot the same object, so their subtypes agree" ); assert_eq!( battlefield_row.core_types, entry_row.core_types, - "both CR 403.3 ledgers snapshot the same object, so their core types agree" + "both CR 608.2i ledgers snapshot the same object, so their core types agree" ); // (3) The deferred emit really happened, carrying the recorder-assigned index (CR 603.6a + // CR 400.7). Read off the `ActionResult` of the copy-target submission itself, which is @@ -705,7 +806,7 @@ fn suppressed_liminal_copy_token_entry_is_recorded_once() { // as a follow-up with the symptom only, not fixed here. } -// ───────── the POSTPONED entry lifecycle (CR 400.7 + CR 403.3 + CR 614.12a) ───────── +// ───────── the POSTPONED entry lifecycle (CR 400.7 + CR 608.2i + CR 614.12a) ───────── // // A `Suppress`-route token is committed to the battlefield BEFORE it is the thing that entered: // `BecomeCopy` has not run, and the copied card's own mandatory as-enters choice (CR 614.12a) is @@ -745,6 +846,17 @@ const FAITHFUL_WATCHDOG_ORACLE: &str = const HARDENED_SCALES_ORACLE: &str = "If one or more +1/+1 counters would be put on a creature you control, that many plus one +1/+1 counters are put on it instead."; const BRANCHING_EVOLUTION_ORACLE: &str = "If one or more +1/+1 counters would be put on a creature you control, twice that many +1/+1 counters are put on that creature instead."; const SOUL_WARDEN_ORACLE: &str = "Whenever another creature enters, you gain 1 life."; +/// Scryfall-verbatim (`Vorinclex, Monstrous Raider`, KHM 199). Used as the ANY-PERMANENT half of +/// the CR 616.1 counter-doubler pair: its doubling clause parses with `valid_card == None`, so it +/// admits an ARTIFACT entrant (an Incubator, an Equipment token) that the creature-scoped Hardened +/// Scales / Branching Evolution pair provably rejects. It carries no token-creation replacement, so +/// the two-token reach batch stays exactly two entries. +const VORINCLEX_ORACLE: &str = "Trample, haste\nIf you would put one or more counters on a permanent or player, put twice that many of each of those kinds of counters on that permanent or player instead.\nIf an opponent would put one or more counters on a permanent or player, they put half that many of each of those kinds of counters on that permanent or player instead, rounded down."; +/// Scryfall-verbatim (`Ozolith, the Shattered Spire`, SOC 281). The `Plus{1}` half of the +/// ANY-PERMANENT pair — `DOUBLE` and `Plus{1}` do not commute, which is what makes the two +/// simultaneously-applicable replacements raise the CR 616.1 ordering prompt the paused fixtures +/// need. Also carries no token-creation replacement. +const OZOLITH_SHATTERED_SPIRE_ORACLE: &str = "If one or more +1/+1 counters would be put on an artifact or creature you control, that many plus one +1/+1 counters are put on it instead.\n{1}{G}, {T}: Put a +1/+1 counter on target artifact or creature you control. Activate only as a sorcery.\nCycling {2} ({2}, Discard this card: Draw a card.)"; /// What one answered prompt did to the token's entry: the events its `ActionResult` carried and /// both per-turn ledgers as of immediately after it returned. @@ -759,7 +871,7 @@ struct CopyEntryStep { tokens_created: usize, /// CR 400.7 rows for the token on `zone_changes_this_turn` after this action. zone_rows: usize, - /// CR 403.3 rows for the token on `battlefield_entries_this_turn` after this action. + /// CR 608.2i rows for the token on `battlefield_entries_this_turn` after this action. entry_rows: usize, /// Whether an entry is still parked awaiting realization after this action. parked: bool, @@ -853,7 +965,7 @@ fn token_entry_step( } /// Activate the graveyard Vizier's Embalm ability and answer every prompt the resulting token -/// entry raises, recording each answer's effect on the two CR 400.7 / CR 403.3 ledgers. +/// entry raises, recording each answer's effect on the two CR 400.7 / CR 608.2i ledgers. /// /// `copy_target` names the battlefield creature the copy-target prompt must pick; `None` DECLINES /// the "enter as a copy" replacement, which routes the entry through `TokenEntryEventEmission::Emit` @@ -994,7 +1106,7 @@ fn entry_rows( .find(|record| record.object_id == token) .unwrap_or_else(|| { panic!( - "the realized copy token must have a CR 403.3 battlefield-entry row; prompts = {:?}", + "the realized copy token must have a CR 608.2i battlefield-entry row; prompts = {:?}", drive.prompts ) }); @@ -1014,7 +1126,7 @@ fn ledger_index(runner: &GameRunner, token: ObjectId) -> usize { .expect("the entry is on the CR 400.7 ledger") } -/// CR 400.7 + CR 403.3 + CR 614.12a — the maintainer's named failure path. Embalm Vizier of Many +/// CR 400.7 + CR 608.2i + CR 614.12a — the maintainer's named failure path. Embalm Vizier of Many /// Faces copying Painter's Servant: the copy carries Painter's MANDATORY "as this creature enters, /// choose a color" replacement, so the entry pauses on a `NamedChoice` that spans a client round /// trip. Both ledgers must describe the REALIZED copy exactly once, and the entry pair must be @@ -1084,7 +1196,7 @@ fn suppressed_liminal_copy_token_entry_realizes_through_a_mandatory_as_enters_ch assert_eq!( (settled.zone_rows, settled.entry_rows), (1, 1), - "the realized entry lands on both CR 400.7 / CR 403.3 ledgers exactly once" + "the realized entry lands on both CR 400.7 / CR 608.2i ledgers exactly once" ); assert!( !settled.parked, @@ -1102,7 +1214,7 @@ fn suppressed_liminal_copy_token_entry_realizes_through_a_mandatory_as_enters_ch ); assert_eq!( battlefield_name, zone_name, - "both CR 403.3 ledgers are written by the one record_zone_change call, so they agree" + "both CR 608.2i ledgers are written by the one record_zone_change call, so they agree" ); // (3) The emit rides the SAME action that realized the entry, exactly once, carrying the @@ -1388,7 +1500,7 @@ fn declined_copy_replacement_records_the_token_entry_without_parking_it() { .filter(|record| record.object_id == token) .count(), 1, - "the Emit-route token is recorded on the CR 403.3 ledger exactly once" + "the Emit-route token is recorded on the CR 608.2i ledger exactly once" ); assert_eq!( life_of_p0(runner.state()) - life_start, @@ -1399,18 +1511,35 @@ fn declined_copy_replacement_records_the_token_entry_without_parking_it() { /// CR 603.2c — a postponed entry must not collide with a normally-recorded one. The realized copy /// token and a plain `Effect::Token` batch minted in the SAME turn (the `Emit` path, through -/// `push_committed_token_entry_events` → `record_committed_token_entry` → `record_zone_change`) -/// must occupy DISTINCT `turn_zone_change_index` values, because the batched zone-change replay -/// guard dedups on that index. +/// `push_committed_token_entry_events` → `zones::record_and_emit_entry_from_no_zone` → +/// `record_zone_change`) must occupy DISTINCT `turn_zone_change_index` values, because the batched +/// zone-change replay guard dedups on that index. /// -/// The second producer is deliberately NOT `token_copy.rs`'s `record_battlefield_entry` sites: -/// those never reach `record_zone_change`, so they have no `zone_changes_this_turn` row to compare -/// against and the assertion would be vacuous. +/// The second producer is the plain `Effect::Token` batch because it is the *pre-existing* routed +/// producer; `token_copy.rs`'s sites are now routed too and get their own dedicated tests (the +/// S4/S5 fixtures at the bottom of this file). This test's subject is the postponed-vs-normal +/// collision, which `mint_token_batch` pins with the fewest moving parts. /// -/// REVERT-PROBE (discriminating, RUN): delete the `record_zone_change` call inside -/// `token::record_committed_token_entry` (push onto `zone_changes_this_turn` directly, leaving the -/// snapshot's `0` placeholder) ⇒ the copy token and the minted tokens all report index `0` and the -/// distinctness assertion fails. +/// REVERT-PROBE, arm 1 — DROP (discriminating, RUN): delete the `record_zone_change` call inside +/// `zones::record_and_emit_entry_from_no_zone` (push onto `zone_changes_this_turn` directly, +/// leaving the snapshot's `0` placeholder) ⇒ every entry ships index `0`. MEASURED failure point: +/// the distinctness assertion below — +/// `the realized copy entry (0) must not share an index with the same-turn token batch ([0, 0])`. +/// +/// REVERT-PROBE, arm 2 — TRIVIALIZE (discriminating, RUN): leave the recorder in place, writing +/// BOTH ledgers' rows, but make `restrictions::record_zone_change` answer a CONSTANT index (`3`) +/// for `from: None → Battlefield` records. Scoping the constant to that arm is required: a global +/// constant is dominated by the ordinary-move `TurnRecordIndexMismatch` invariant +/// (`zones.rs`'s `expect("ordinary zone transition must install its resolved core")`), which +/// panics before this test's own assertions. MEASURED failure point: the same assertion — +/// `the realized copy entry (3) must not share an index with the same-turn token batch ([3, 3])`. +/// A recorder that records but does not COUNT is what arm 1 alone cannot see. +/// +/// MEASURED CONTROL for both arms: the pre-fix form of this test read the copy side through +/// `ledger_index` — a ledger POSITION compared against event-borne STORED indices — and reported +/// `ok` under arm 1 (position `1` vs `[0, 0]`) AND under arm 2 (position `1` vs `[3, 3]`). It +/// could not fail for the index-`0` class it names. That is why the copy side is read off its own +/// emitted event below. #[test] fn a_realized_copy_token_entry_and_a_same_turn_token_batch_take_distinct_indices() { let mut scenario = GameScenario::new(); @@ -1431,8 +1560,27 @@ fn a_realized_copy_token_entry_and_a_same_turn_token_batch_take_distinct_indices "NamedChoice(5)".to_string(), ], ); - let token = drive.token(); - let copy_index = ledger_index(&runner, token); + // STORED-vs-STORED, deliberately NOT `ledger_index`. The CR 603.2c dedup guard + // (`triggers.rs::batched_zone_change_already_collected`) keys on the + // `turn_zone_change_index` it reads off the EVENT, so both sides of the distinctness claim + // must be event-borne stored indices. `ledger_index` returns a ledger POSITION, which equals + // the stored index only when every row was recorded correctly — i.e. exactly when the defect + // is absent. MEASURED: with a position on the copy side this assertion survived its own + // revert probe (copy POSITION 1 vs minted STORED [0, 0] ⇒ `all(!= 1)` holds), so it could not + // fail for the index-`0` placeholder class it names. + // + // The `drive.token()` reach-guard this replaced is carried structurally: `token_entry_step` + // filters `zone_changed_indices` by the drive's own token id, so a drive that never reached + // the copy-target prompt yields an EMPTY vec and fails the length assertion below. + let copy_indices = &drive.steps[2].zone_changed_indices; + assert_eq!( + copy_indices.len(), + 1, + "the realizing action emits exactly one battlefield ZoneChanged for the copy token; \ + prompts = {:?}", + drive.prompts + ); + let copy_index = copy_indices[0]; let turn_start = runner.state().turn_number; let minted = mint_token_batch(runner.state_mut(), painter, 2); @@ -1512,3 +1660,1201 @@ fn unpaused_copy_token_entry_is_realized_by_the_copy_target_action_itself() { assert_eq!(zone_power, Some(2)); assert_eq!(battlefield_name, zone_name); } + +// ═════════════════════════════════════════════════════════════════════════════════════════════ +// CR 400.7 + CR 608.2i + CR 603.2c — the SIX remaining battlefield-entry emit sites. +// +// `snapshot_for_zone_change` leaves `turn_zone_change_index` at its `0` placeholder for +// `restrictions::record_zone_change` to overwrite. Six production sites (seven fix points) built +// the record and emitted it WITHOUT ever reaching the recorder, so each shipped index `0`: +// S1 `conjure.rs` — conjure onto the battlefield +// S2 `counters.rs` InjectPredefinedTokenAbilities — incubate resumed past a counter pause +// S3a `counters.rs` FinalizeTokenEntry — spec token resumed past a counter pause +// S3b `counters.rs` FinalizeCopyTokenEntry — copy token resumed past a counter pause +// S4 `token_copy.rs` copy-loop tail — ordinary copy token +// S5 `token_copy.rs` modification-pause resume +// S6 `gift_delivery.rs` create_gift_token — gift Treasure/Food/Fish/Card tokens +// +// THE DISCRIMINATING DIRECTION IS DICTATED BY THE GUARD. +// `triggers.rs::batched_zone_change_already_collected` suppresses only when EVERY index in the +// candidate batch is already in `batched_zone_change_trigger_fired`. So the site's entry must be +// driven SECOND, after a routed two-token batch has already collected `(def, 0)` and `(def, 1)`: +// the unrouted site then offers the single-element list `[0]`, `all()` is true, and its fire is +// swallowed (life delta stays 1). Routed, it offers `[2]`, which is not in the set, and it fires +// (life delta 2). Site FIRST would NOT discriminate — the batch's later entries carry uncollected +// indices, so `all()` is false and both trees fire. Same idiom as +// `mixed_group_sibling_last_also_fires` above. +// ═════════════════════════════════════════════════════════════════════════════════════════════ + +/// Run the real trigger pipeline over the events a directly-driven resolver produced — the same +/// two-line tail `mint_token_batch` and `incubate_batch` run inline. +fn settle(state: &mut GameState, events: &[GameEvent]) { + process_triggers(state, events); + drain_order_triggers_with_identity(state); +} + +/// What one site fixture measured. +struct SiteRun { + /// The permanent the SITE's own drive put onto the battlefield. + site_obj: ObjectId, + /// `turn_zone_change_index` of every battlefield `ZoneChanged` the SITE's drive emitted. + site_indices: Vec, + /// P0's life change across the whole fixture (reach batch + site entry). + life_delta: i32, + /// The reach batch's indices — the negative sibling: the already-correct `token.rs` route + /// must be untouched by this change. + batch_indices: Vec, +} + +/// The single battlefield `ZoneChanged` the SITE's drive emitted: which object entered, and the +/// index the event actually shipped. Reads on BOTH trees — the unrouted sites still emit the +/// event, just carrying the `0` placeholder — which is what makes the index a discriminator +/// rather than a presence check. +fn site_entry(events: &[GameEvent], what: &str) -> (ObjectId, Vec) { + let entries: Vec<(ObjectId, usize)> = events + .iter() + .filter_map(|event| match event { + GameEvent::ZoneChanged { + object_id, + to: Zone::Battlefield, + record, + .. + } => Some((*object_id, record.turn_zone_change_index)), + _ => None, + }) + .collect(); + assert_eq!( + entries.len(), + 1, + "{what} must emit exactly ONE battlefield ZoneChanged for its own entry \ + (reach-guard: a fixture that never reached its site emits none); got {entries:?}" + ); + (entries[0].0, vec![entries[0].1]) +} + +fn install_host_trigger(runner: &mut GameRunner, host: ObjectId) { + runner + .state_mut() + .objects + .get_mut(&host) + .expect("host permanent") + .trigger_definitions + .push(batched_etb_life_trigger()); +} + +/// Open the turn under measurement and, when `mint_batch`, drive the routed two-token batch that +/// collects `(def, 0)` and `(def, 1)` — the reach batch every discriminator needs so its own +/// entry is the SECOND occurrence. +fn open_turn(runner: &mut GameRunner, host: ObjectId, mint_batch: bool) -> (i32, u32, Vec) { + assert_eq!( + runner.state().zone_changes_this_turn.len(), + 0, + "legibility: scenario staging must leave the per-turn zone-change ledger empty, so the \ + indices below are the fixture's own" + ); + let life_start = life_of_p0(runner.state()); + let turn_start = runner.state().turn_number; + let batch_indices = if mint_batch { + let batch = mint_token_batch(runner.state_mut(), host, 2); + runner.advance_until_stack_empty(); + // POSITIVE reach-guard: without this the site's "unchanged total" below would be a + // fixture that never triggered rather than a genuine suppression. + assert_eq!( + life_of_p0(runner.state()) - life_start, + 1, + "the two-token reach batch fires the batched trigger exactly ONCE (CR 603.2c)" + ); + zone_change_indices(&batch) + } else { + Vec::new() + }; + (life_start, turn_start, batch_indices) +} + +/// Answer the CR 616.1 counter-ordering pause a directly-driven resolver parked, through the REAL +/// engine dispatch: `runner.act` routes into `engine_replacement::handle_replacement_choice` plus +/// the post-action pipeline, which runs the trigger scan itself — hence no `settle` here. Shipped +/// idiom for "direct resolver parks `waiting_for`, then `runner.act(ChooseReplacement)`": +/// `counter_double_redirect_choice.rs`. +fn answer_counter_order(runner: &mut GameRunner, what: &str) -> Vec { + let WaitingFor::ReplacementChoice { candidates, .. } = runner.state().waiting_for.clone() + else { + panic!( + "{what} must park the CR 616.1 counter-ordering choice, got {:?}", + runner.state().waiting_for + ); + }; + assert_eq!( + candidates.len(), + 2, + "{what}: a 1-candidate prompt is an optional replacement, not the CR 616.1 ordering pause" + ); + let result = runner + .act(GameAction::ChooseReplacement { index: 0 }) + .expect("answer the CR 616.1 ordering prompt"); + let events = result.events; + runner.advance_until_stack_empty(); + events +} + +/// M1–M4 plus the paired-deletion must-not-flip and the T9 negative sibling — the mechanism half +/// of every discriminator. NOT called by the T8 controls (their whole job is to read identically +/// on both trees). +fn assert_site_records(runner: &GameRunner, run: &SiteRun, expected_index: usize) { + // M1 — ledger self-consistency. Every production push now routes through `record_zone_change`, + // which sets index = position by construction. Unfixed at S1/S2 the direct `push_back` lands a + // row at position 2 still carrying the placeholder `0`. + assert!( + runner + .state() + .zone_changes_this_turn + .iter() + .enumerate() + .all(|(position, record)| record.turn_zone_change_index == position), + "every zone-change row's index must equal its position — a direct `push_back` that skips \ + `record_zone_change` lands a row carrying the `0` placeholder; ledger = {:?}", + runner + .state() + .zone_changes_this_turn + .iter() + .map(|record| (record.object_id, record.turn_zone_change_index)) + .collect::>() + ); + + // M2 — row presence. The five sites that never reached the recorder pushed NO row at all. + assert_eq!( + runner + .state() + .zone_changes_this_turn + .iter() + .filter(|record| record.object_id == run.site_obj + && record.to_zone == Zone::Battlefield) + .count(), + 1, + "the site's entry is on the CR 400.7 ledger exactly once (unrouted ⇒ 0; a kept direct \ + push alongside the recorder ⇒ 2)" + ); + + // M3 — the EMITTED index, the dedup key `triggers.rs` reads off the event. + assert_eq!( + run.site_indices, + vec![expected_index], + "the emitted ZoneChanged carries the index the recorder assigned (placeholder ⇒ [0])" + ); + + // M4 — subscript contract. `ability.rs::self_ref_own_departure_successor` uses this index as a + // ROW SUBSCRIPT into `zone_changes_this_turn`; unfixed it subscripts row 0, which is the reach + // batch's first Saproling, not the emitting object. + assert_eq!( + runner.state().zone_changes_this_turn[run.site_indices[0]].object_id, + run.site_obj, + "the emitted index must subscript the EMITTING object's own ledger row" + ); + + // MUST-NOT-FLIP FOR THE PAIRED DELETION — this is the assertion that carries that claim, and + // it carries it because these sites are the ones the deletion actually touched. Measured on + // `4b34e5465`, the pre-change `record_battlefield_entry` call sites outside `restrictions.rs` + // are `conjure.rs:191`, `counters.rs:518/558/637`, `gift_delivery.rs:157` and + // `token_copy.rs:851/969` — the routes `SiteRun` drives. `record_zone_change` performs the + // CR 608.2i bookkeeping itself, so re-adding any one of those deleted calls makes this 2. + // MEASURED: re-adding `gift_delivery.rs:157` fails this assertion, `left: 2 right: 1`. + assert_eq!( + runner + .state() + .battlefield_entries_this_turn + .iter() + .filter(|record| record.object_id == run.site_obj) + .count(), + 1, + "the site's entry is on the CR 608.2i ledger exactly once (re-adding the deleted \ + `record_battlefield_entry` ⇒ 2)" + ); + + // T9 NEGATIVE SIBLING: the already-correct `token.rs` route is untouched. Neither counter + // doubler replaces token creation, so the reach batch is exactly two entries on both trees. + assert_eq!( + run.batch_indices, + vec![0, 1], + "the reach batch keeps its own two legitimate indices" + ); +} + +/// Both doublers of the CREATURE pair. Their `valid_card` is `Typed{[Creature], You}`, so they +/// admit a creature entrant only. +fn stage_creature_counter_pair(scenario: &mut GameScenario) { + scenario.add_enchantment_from_oracle(P0, "Hardened Scales", HARDENED_SCALES_ORACLE); + scenario.add_enchantment_from_oracle(P0, "Branching Evolution", BRANCHING_EVOLUTION_ORACLE); +} + +/// Both doublers of the ANY-PERMANENT pair — required whenever the entrant is an ARTIFACT (the +/// Incubator, the Equipment token), which the creature pair provably rejects. Vorinclex's +/// doubling clause parses with `valid_card == None`; Ozolith's admits `artifact or creature you +/// control`. `add_enchantment_from_oracle` stands in for the missing artifact builder: replacement +/// candidacy gates the SOURCE only on its zone, and `valid_card` filters the AFFECTED object. +fn stage_any_permanent_counter_pair(scenario: &mut GameScenario) { + scenario.add_creature_from_oracle(P0, "Vorinclex, Monstrous Raider", 6, 6, VORINCLEX_ORACLE); + scenario.add_enchantment_from_oracle( + P0, + "Ozolith, the Shattered Spire", + OZOLITH_SHATTERED_SPIRE_ORACLE, + ); +} + +fn copy_token_effect(additional_modifications: Vec) -> Effect { + Effect::CopyTokenOf { + target: TargetFilter::Any, + owner: TargetFilter::Controller, + source_filter: None, + enters_attacking: false, + tapped: false, + count: QuantityExpr::Fixed { value: 1 }, + extra_keywords: Vec::new(), + additional_modifications, + } +} + +// ── S1 · conjure.rs ────────────────────────────────────────────────────────────────────────── + +fn drive_s1(mint_batch: bool) -> (GameRunner, SiteRun) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let host = scenario.add_creature(P0, "Batched Watcher", 1, 1).id(); + let mut runner = scenario.build(); + install_host_trigger(&mut runner, host); + + let (life_start, turn_start, batch_indices) = open_turn(&mut runner, host, mint_batch); + + // A missing card-registry entry is harmless here: `ConjuredIdentity::Named { face: None }` + // still creates the object and still takes the `destination == Battlefield` arm. + let ability = ResolvedAbility::new( + Effect::Conjure { + cards: vec![ConjureCard { + source: ConjureSource::Named { + name: "Verdant Dread".to_string(), + }, + count: QuantityExpr::Fixed { value: 1 }, + }], + destination: Zone::Battlefield, + tapped: false, + library_position: None, + library_players: None, + }, + Vec::new(), + host, + P0, + ); + let mut events = Vec::new(); + conjure::resolve(runner.state_mut(), &ability, &mut events).expect("the conjure resolves"); + let (site_obj, site_indices) = site_entry(&events, "the battlefield conjure"); + settle(runner.state_mut(), &events); + runner.advance_until_stack_empty(); + + assert_eq!( + runner.state().turn_number, + turn_start, + "the whole fixture is ONE turn (both ledgers are per-turn)" + ); + let life_delta = life_of_p0(runner.state()) - life_start; + ( + runner, + SiteRun { + site_obj, + site_indices, + life_delta, + batch_indices, + }, + ) +} + +/// S1 (CR 603.6a + CR 603.2c): a conjured battlefield entry driven AFTER a routed token batch is a +/// distinct occurrence and must fire the batched ETB trigger again. +/// +/// REVERT-PROBE (discriminating, RUN): replace the +/// `zones::record_and_emit_entry_from_no_zone` call at `conjure.rs` with the hand-rolled +/// `snapshot_for_zone_change` + `state.zone_changes_this_turn.push_back(…)` + +/// `events.push(ZoneChanged)` ⇒ the conjured entry ships the `0` placeholder, collides +/// with the batch's already-collected `(def, 0)`, its fire is swallowed, and the life delta reads +/// 1 instead of 2 (M1, M3 and M4 fail with it). +#[test] +fn conjured_battlefield_entry_after_a_token_batch_fires_the_batched_trigger() { + let (runner, run) = drive_s1(true); + assert_eq!( + run.life_delta, 2, + "the conjured entry after a token batch fires the batched trigger AGAIN \ + (index-0 placeholder ⇒ collides with the batch's legitimate 0 ⇒ 1)" + ); + assert_site_records(&runner, &run, 2); +} + +// ── S6 · gift_delivery.rs ──────────────────────────────────────────────────────────────────── + +/// Consumer 1's PRODUCTION seam: `quantity.rs`'s `ZoneChangeCountThisTurn` population scan, driven +/// through the real resolver `game::quantity::resolve_quantity`. `TargetFilter::Any` reaches +/// `zone_change_filter_inner`'s `Any => true` arm, so no filter conjunct can dominate the answer, +/// and `source` is only the filter-context source id — which `Any` ignores. That is what makes the +/// same query legible at a point where the site object does not exist yet. +fn zone_change_count_this_turn(runner: &GameRunner, source: ObjectId) -> i32 { + resolve_quantity( + runner.state(), + &QuantityExpr::Ref { + qty: QuantityRef::ZoneChangeCountThisTurn { + from: None, + to: Some(Zone::Battlefield), + filter: TargetFilter::Any, + }, + }, + P0, + source, + ) +} + +/// The battlefield `ZoneChanged` record the SITE's own drive emitted for `site_obj`. +fn site_entry_record( + events: &[GameEvent], + site_obj: ObjectId, +) -> engine::types::game_state::ZoneChangeRecord { + events + .iter() + .find_map(|event| match event { + GameEvent::ZoneChanged { + object_id, + to: Zone::Battlefield, + record, + .. + } if *object_id == site_obj => Some((**record).clone()), + _ => None, + }) + .expect("the site's own battlefield ZoneChanged") +} + +/// `drive_s6` with its two observation points exposed, so tests **H** and **L** can read the state +/// a monolithic drive hides: the trigger host, consumer 1's answer BEFORE the site enters, and the +/// site's own emitted events (which carry the `ZoneChangeRecord` that went on the wire). One body, +/// so the S6 discriminator, its T8 control, H and L all run the identical fixture. +fn drive_s6_observed(mint_batch: bool) -> (GameRunner, SiteRun, Vec, ObjectId, i32) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let host = scenario.add_creature(P0, "Batched Watcher", 1, 1).id(); + let mut runner = scenario.build(); + install_host_trigger(&mut runner, host); + + let (life_start, turn_start, batch_indices) = open_turn(&mut runner, host, mint_batch); + + // OBSERVATION POINT (1) — consumer 1's seam AFTER the priming batch, BEFORE the site's entry. + // The site object does not exist yet, so the host is the filter-context source at both points + // and the two calls are literally the same query. + let before = zone_change_count_this_turn(&runner, host); + + // Both context fields are load-bearing: `resolve` no-ops without `additional_cost_paid`, and + // returns early without a latched recipient (CR 702.174a). Same shape as the in-repo + // `make_gift_ability`. + let mut ability = ResolvedAbility::new( + Effect::GiftDelivery { + kind: GiftKind::Treasure, + }, + Vec::new(), + host, + P0, + ); + ability.context.additional_cost_paid = true; + ability.context.gift_recipient = Some(PlayerId(1)); + + let mut events = Vec::new(); + gift_delivery::resolve(runner.state_mut(), &ability, &mut events) + .expect("the gift delivery resolves"); + // Reach-guard: the Treasure was actually created. The batched trigger has no `valid_card`, so + // P1's token still fires it and its `GainLife { player: Controller }` still pays P0. + assert!( + events + .iter() + .any(|event| matches!(event, GameEvent::TokenCreated { .. })), + "the promised gift must create the Treasure token" + ); + let (site_obj, site_indices) = site_entry(&events, "the gift token"); + settle(runner.state_mut(), &events); + runner.advance_until_stack_empty(); + + assert_eq!(runner.state().turn_number, turn_start, "one turn"); + let life_delta = life_of_p0(runner.state()) - life_start; + ( + runner, + SiteRun { + site_obj, + site_indices, + life_delta, + batch_indices, + }, + events, + host, + before, + ) +} + +fn drive_s6(mint_batch: bool) -> (GameRunner, SiteRun) { + let (runner, run, _events, _host, _before) = drive_s6_observed(mint_batch); + (runner, run) +} + +/// **H** — CR 400.7 + CR 603.6a, the event↔ledger IDENTITY agreement (requirement R3). +/// +/// `Ability::self_ref_own_departure_successor` (`types/ability.rs`) uses the index the EVENT +/// carries as a SUBSCRIPT into `state.zone_changes_this_turn` and then requires the row it lands on +/// to carry the SAME `trigger_source_context().identity.reference` as the event's own record. An +/// entry that emits without recording ships the `0` placeholder, so that subscript lands on a row +/// belonging to a DIFFERENT object and the `SelfRef` binding silently fails. +/// +/// This asserts R3, not `Some`-ness: `trigger_source_context` is `Some` on BOTH trees by +/// construction (`game_object.rs`'s snapshot builds it unconditionally), so an +/// `assert!(…is_some())` here would be a snapshot of the constructor and could never fail. The +/// payload is assertion (2). +/// +/// VACUITY: the priming batch is MANDATORY. With an empty ledger the site's index would legally be +/// `0` and row `0` would be the site's own row, so (2) would pass pre-fix for the wrong reason. +/// The batch-first ordering is what makes H discriminating, exactly as for T1–T7; the +/// `batch_indices == [0, 1]` guard below pins it. +/// +/// REVERT-PROBE (discriminating, RUN — this is PROBE X): revert the SITE, not the shared +/// authority. In `gift_delivery.rs::create_gift_token`, replace the +/// `token::push_committed_token_entry_events` call with the hand-rolled pair — +/// `record_battlefield_entry`, +/// `zone_changes_this_turn.push_back(snapshot_for_zone_change(obj, None, Battlefield))`, then +/// `events.push(ZoneChanged { .. })` + `events.push(TokenCreated { .. })` — so the site's row +/// EXISTS but keeps the `0` placeholder index. +/// +/// Site-isolated, not authority-wide, and that is load-bearing: reverting the shared authority +/// also strips the PRIMING batch's rows, so this test dies at the vacuity guard above +/// (MEASURED: `left: [0, 0] right: [0, 1]` at the `batch_indices` assertion) and assertion (2) is +/// never evaluated. The site-isolated form leaves the priming batch on the real authority, so the +/// guard passes and the payload is what flips. +/// +/// MEASURED, site-isolated: `idx == 0`, the ledger row at `0` is the priming batch's first +/// Saproling, and assertion (2) fails — +/// `left: ObjectIncarnationRef { object_id: ObjectId(2), incarnation: 1 }`, +/// `right: ObjectIncarnationRef { object_id: ObjectId(5), incarnation: 1 }`, +/// ledger `[(ObjectId(2), 0), (ObjectId(3), 1), (ObjectId(5), 0)]`. +/// MEASURED on the same probe build: **L** and its folded L-b assertions stay GREEN — that +/// contrast is what proves H's payload is the index/identity while L's is row presence. +#[test] +fn the_site_row_the_event_subscripts_carries_the_entering_objects_identity() { + let (runner, run, events, _host, _before) = drive_s6_observed(true); + + // Vacuity guard, before touching the site row: the priming batch really did take 0 and 1. + assert_eq!( + run.batch_indices, + vec![0, 1], + "H is only discriminating when the site's entry is the SECOND occurrence — the priming \ + batch must own indices 0 and 1" + ); + + let record = site_entry_record(&events, run.site_obj); + let idx = record.turn_zone_change_index; + + // (1) PRECONDITION R1 — an `.expect`, deliberately not an `assert!(…is_some())`: this is a + // precondition of the payload, and it holds on both trees. + let ev_ctx = record + .trigger_source_context() + .expect("a real zone-change event carries its source context"); + + // (2) PAYLOAD — R3. The ledger row the EVENT subscripts is the row that event wrote. + assert_eq!( + runner.state().zone_changes_this_turn[idx] + .trigger_source_context() + .expect("the recorded row carries its source context") + .identity + .reference, + ev_ctx.identity.reference, + "CR 400.7: the ledger row at the event's own `turn_zone_change_index` must be the record \ + that event wrote — an unrecorded entry ships the `0` placeholder and subscripts a row \ + belonging to a different object (ledger = {:?})", + runner + .state() + .zone_changes_this_turn + .iter() + .map(|row| (row.object_id, row.turn_zone_change_index)) + .collect::>() + ); + + // (3) R2 — the identity is the entering object's live incarnation. + assert_eq!(ev_ctx.identity.reference.object_id, run.site_obj); + assert_eq!( + ev_ctx.identity.reference.incarnation, + runner + .state() + .objects + .get(&run.site_obj) + .expect("the gift token is on the battlefield") + .incarnation + ); + + // (4) R4 — free and NON-DISCRIMINATING, labelled as such: `ObjectIdentityBinding::new(…, + // from.unwrap_or(self.zone))` falls back to the live zone when `from` is `None`, and that + // fallback is independently guaranteed by `record_battlefield_entry`'s + // `obj.zone != Battlefield → return` guard, which `assert_site_records`'s must-not-flip clause + // already exercises. It passes on both trees. + assert_eq!(ev_ctx.identity.expected_zone, Zone::Battlefield); +} + +/// **L** (with **L-b** folded in) — CR 400.7: the new ledger row is visible at the two production +/// seams that read `zone_changes_this_turn` by content rather than by subscript. +/// +/// **L** drives consumer 1, `quantity.rs`'s `ZoneChangeCountThisTurn` population scan, through the +/// real resolver at TWO points of ONE run: after the priming batch (`before`) and after the site's +/// drive (`after`). The assertion is a DELTA (`after == before + 1`), never an absolute — the +/// primed fixture is what gives L-b its cross-producer property, and a delta cannot be invalidated +/// by a change in how many rows the priming batch mints. +/// +/// **L-b** then runs consumer 4's own seam on the same post-drive state: +/// `filter::matches_target_filter_on_zone_change_record` with `TargetFilter::SelfRef` and a +/// `FilterContext::from_trigger_source` built from the site event's context. Three +/// `(None, Battlefield)` rows from TWO producers are on the ledger; the seam must select exactly +/// the site's. +/// +/// ZERO-CENSUS POSITIVE CONTROL (mandatory): `before > 0` is asserted in the same run. An +/// instrument that can only ever answer `0` proves nothing about an absence. +/// +/// DOMINATING-CONJUNCT CHECK: `TargetFilter::Any` reaches `zone_change_filter_inner`'s +/// `Any => true`, and the `from`/`to` conjuncts are `is_none_or`, so nothing upstream of the row +/// can dominate L's answer. `matches_target_filter_on_zone_change_record` is a pure pass-through to +/// `zone_change_filter_inner`, so nothing sits between the row and the `SelfRef` arm for L-b. +/// +/// DISCLOSED NON-DISCRIMINATION (do not upgrade this claim): `FilterContext::from_trigger_source` +/// sets `source_id` from the same identity, so the `SelfRef` arm's `map_or` fallback would select +/// the same row. L-b measures ADMISSION at consumer 4's seam, not identity-vs-ObjectId inside the +/// arm. The identity payload is H's job. +/// +/// REVERT-PROBE (discriminating, RUN — this is PROBE Y): revert the SITE, not the shared +/// authority. In `gift_delivery.rs::create_gift_token`, replace the +/// `token::push_committed_token_entry_events` call with `events.push(ZoneChanged { .. })` + +/// `events.push(TokenCreated { .. })` over a bare `snapshot_for_zone_change` and NO recording at +/// all — no `push_back`, no `record_battlefield_entry`, index left at its `0` placeholder. +/// +/// Site-isolated, not authority-wide, and that is load-bearing: reverting the shared authority +/// also strips the PRIMING batch's rows, so the zero-census positive control above fails first +/// (`before` collapses to `0`) and the fixture does NOT stay intact. The site-isolated form leaves +/// the priming batch on the real authority, so `before` stays non-zero and the delta is what flips. +/// +/// MEASURED, site-isolated: the DELTA assertion fails — `left: 2 right: 3`, `before = 2, +/// after = 2` — with the `before > 0` control green. The L-b `selected` assertion is NOT reached +/// (the delta assertion dominates), so this probe pins row PRESENCE at consumer 1's seam only; +/// no claim is made here about L-b flipping. PROBE X — the site's row still pushed, index left at +/// `0` — leaves this test GREEN (measured); that is H's probe, not L's. +#[test] +fn zone_change_count_this_turn_sees_the_gift_token_entry() { + let (runner, run, events, host, before) = drive_s6_observed(true); + + // Zero-census positive control: the instrument answers non-zero before the site ever runs. + assert!( + before > 0, + "the priming batch must already be visible at consumer 1's seam — an instrument stuck at \ + 0 could not distinguish 'the site added nothing' from 'the query sees nothing'" + ); + + let after = zone_change_count_this_turn(&runner, host); + assert_eq!( + after, + before + 1, + "CR 400.7: the gift token's battlefield entry must add exactly one row that consumer 1's \ + production scan can see (before = {before}, after = {after})" + ); + + // L-b — consumer 4 (`filter.rs`'s `TargetFilter::SelfRef` arm) at its own production seam. + let record = site_entry_record(&events, run.site_obj); + let ev_ctx = record + .trigger_source_context() + .expect("a real zone-change event carries its source context"); + let ctx = FilterContext::from_trigger_source(ev_ctx); + let select_with = |filter: &TargetFilter| -> Vec { + runner + .state() + .zone_changes_this_turn + .iter() + .filter(|row| { + matches_target_filter_on_zone_change_record(runner.state(), row, filter, &ctx) + }) + .map(|row| row.object_id) + .collect() + }; + + // REACHABILITY EXHIBIT for the exclusion below. `selected == vec![site_obj]` is an asserted + // NEGATIVE about the two priming rows, and a negative is vacuous if the excluded rows could + // never have been selected in the first place. Run the SAME loop over the SAME ledger with an + // admitting filter first: all three rows ARE reachable at this seam, so the exclusion is the + // `SelfRef` arm's doing and not an artefact of the population. + let reachable = select_with(&TargetFilter::Any); + assert_eq!( + reachable.len(), + 3, + "reachability exhibit: with an admitting filter this seam selects all three \ + `(None, Battlefield)` rows this turn — two priming Saprolings and the site's own. \ + Without that, the SelfRef exclusion below would be a negative about rows that were never \ + selectable; got {reachable:?}" + ); + assert!( + reachable.contains(&run.site_obj), + "reachability exhibit: the site's own row must be among them; got {reachable:?}" + ); + + let selected = select_with(&TargetFilter::SelfRef); + assert_eq!( + selected, + vec![run.site_obj], + "CR 400.7: consumer 4's SelfRef seam must select exactly the site's own row out of the \ + three `(None, Battlefield)` rows two different producers wrote this turn (all three are \ + reachable here — see the exhibit above)" + ); +} + +/// S6 (CR 111.1 + CR 603.6a): a gift token entering after a routed token batch fires the batched +/// ETB trigger again. +/// +/// REVERT-PROBE (discriminating, RUN): restore `record_battlefield_entry` plus the +/// snapshot/`ZoneChanged`/`TokenCreated` block in `gift_delivery.rs::create_gift_token` in place +/// of `push_committed_token_entry_events` ⇒ life delta 2→1, M2 1→0, M3 [2]→[0], M4 fails. +#[test] +fn gift_token_entry_after_a_token_batch_fires_the_batched_trigger() { + let (runner, run) = drive_s6(true); + assert_eq!( + run.life_delta, 2, + "the gift token entry after a token batch fires the batched trigger AGAIN" + ); + assert_site_records(&runner, &run, 2); +} + +// ── S4 · token_copy.rs copy-loop tail ──────────────────────────────────────────────────────── + +fn drive_s4(mint_batch: bool) -> (GameRunner, SiteRun) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let host = scenario.add_creature(P0, "Batched Watcher", 1, 1).id(); + // The Watchdog's "enters with three +1/+1 counters" seeds `etb_counters`, which forces the + // NON-liminal copy branch. With NO doubler on board the counter addition executes without + // pausing, so the loop falls through to the S4 tail. + let watchdog = scenario + .add_creature(P0, "Faithful Watchdog", 0, 0) + .with_plus_counters(3) + .from_oracle_text_with_keywords(&["Vigilance"], FAITHFUL_WATCHDOG_ORACLE) + .id(); + let mut runner = scenario.build(); + install_host_trigger(&mut runner, host); + + let (life_start, turn_start, batch_indices) = open_turn(&mut runner, host, mint_batch); + + let ability = ResolvedAbility::new( + copy_token_effect(Vec::new()), + vec![TargetRef::Object(watchdog)], + host, + P0, + ); + let mut events = Vec::new(); + token_copy::resolve(runner.state_mut(), &ability, &mut events).expect("the copy resolves"); + assert!( + events + .iter() + .any(|event| matches!(event, GameEvent::TokenCreated { .. })), + "the copy token must be created" + ); + let (site_obj, site_indices) = site_entry(&events, "the copy-loop tail"); + + // POSITIVE CONTROL for the `etb_counters` seed that routes this fixture (and T6's) into the + // non-liminal branch: if this reads 0 the seed never materialized and the S3b fixture's + // premise is dead too. Declared escalation, not a degradable assertion. + assert_eq!( + runner.state().objects[&site_obj] + .counters + .get(&CounterType::Plus1Plus1) + .copied() + .unwrap_or(0), + 3, + "the copy carries the Watchdog's three +1/+1 counters — this is what seeds `etb_counters` \ + and forces the non-liminal branch" + ); + + settle(runner.state_mut(), &events); + runner.advance_until_stack_empty(); + + assert_eq!(runner.state().turn_number, turn_start, "one turn"); + let life_delta = life_of_p0(runner.state()) - life_start; + ( + runner, + SiteRun { + site_obj, + site_indices, + life_delta, + batch_indices, + }, + ) +} + +/// S4 (CR 707.2 + CR 603.6a): an ordinary copy token's entry after a routed token batch fires the +/// batched ETB trigger again. +/// +/// REVERT-PROBE (discriminating, RUN): restore `record_battlefield_entry` plus the +/// snapshot/`ZoneChanged`/`TokenCreated` block at the `token_copy.rs` copy-loop tail ⇒ life delta +/// 2→1, M2 1→0, M3 [2]→[0], M4 fails. +#[test] +fn copy_token_tail_entry_after_a_token_batch_fires_the_batched_trigger() { + let (runner, run) = drive_s4(true); + assert_eq!( + run.life_delta, 2, + "the copy token's entry after a token batch fires the batched trigger AGAIN" + ); + assert_site_records(&runner, &run, 2); +} + +// ── S5 · token_copy.rs modification-pause resume ───────────────────────────────────────────── + +fn drive_s5(mint_batch: bool) -> (GameRunner, SiteRun) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let host = scenario.add_creature(P0, "Batched Watcher", 1, 1).id(); + // Vanilla source: it seeds NO `etb_counters`, so the only pausable stage is + // `apply_token_modifications` — which is the S5 resume, not S3b's. + let bears = scenario.add_creature(P0, "Grizzly Bears", 2, 2).id(); + // The copy is a CREATURE, so the creature-scoped doubler pair admits it. + stage_creature_counter_pair(&mut scenario); + let mut runner = scenario.build(); + install_host_trigger(&mut runner, host); + + let (life_start, turn_start, batch_indices) = open_turn(&mut runner, host, mint_batch); + + // `AddCounterOnEnter` is NOT liminal-immediate, so the non-liminal branch runs regardless of + // `etb_counters`; its counter addition meets two competing replacements and parks CR 616.1. + let ability = ResolvedAbility::new( + copy_token_effect(vec![ContinuousModification::AddCounterOnEnter { + counter_type: CounterType::Plus1Plus1, + count: QuantityExpr::Fixed { value: 1 }, + if_type: None, + }]), + vec![TargetRef::Object(bears)], + host, + P0, + ); + let mut events = Vec::new(); + token_copy::resolve(runner.state_mut(), &ability, &mut events).expect("the copy resolves"); + let resume_events = answer_counter_order(&mut runner, "the copy's AddCounterOnEnter"); + let (site_obj, site_indices) = site_entry(&resume_events, "the modification-pause resume"); + + assert_eq!(runner.state().turn_number, turn_start, "one turn"); + let life_delta = life_of_p0(runner.state()) - life_start; + ( + runner, + SiteRun { + site_obj, + site_indices, + life_delta, + batch_indices, + }, + ) +} + +/// S5 (CR 616.1 + CR 603.6a): a copy token whose entry was postponed across a counter-ordering +/// pause fires the batched ETB trigger when it finally enters after a routed token batch. +/// +/// REVERT-PROBE (discriminating, RUN): restore `record_battlefield_entry` plus the +/// snapshot/`ZoneChanged`/`TokenCreated` block in +/// `token_copy.rs::apply_remaining_token_modifications_after_counter_pause` ⇒ life delta 2→1, +/// M2 1→0, M3 [2]→[0], M4 fails. +#[test] +fn modification_paused_copy_token_entry_after_a_token_batch_fires_the_batched_trigger() { + let (runner, run) = drive_s5(true); + assert_eq!( + run.life_delta, 2, + "the modification-paused copy's entry fires the batched trigger AGAIN" + ); + assert_site_records(&runner, &run, 2); +} + +// ── S3a · counters.rs FinalizeTokenEntry (unattached) ──────────────────────────────────────── + +fn spec_token_effect(types: Vec, attach_to: Option, name: &str) -> Effect { + Effect::Token { + name: name.to_string(), + power: PtValue::Fixed(0), + toughness: PtValue::Fixed(0), + types, + colors: Vec::new(), + keywords: Vec::new(), + tapped: false, + count: QuantityExpr::Fixed { value: 1 }, + owner: TargetFilter::Controller, + attach_to, + enters_attacking: false, + supertypes: Vec::new(), + static_abilities: Vec::new(), + enter_with_counters: vec![(CounterType::Plus1Plus1, QuantityExpr::Fixed { value: 1 })], + } +} + +fn drive_s3a_unattached(mint_batch: bool) -> (GameRunner, SiteRun) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let host = scenario.add_creature(P0, "Batched Watcher", 1, 1).id(); + stage_creature_counter_pair(&mut scenario); + let mut runner = scenario.build(); + install_host_trigger(&mut runner, host); + + let (life_start, turn_start, batch_indices) = open_turn(&mut runner, host, mint_batch); + + // A CREATURE token with `enter_with_counters`: the two competing +1/+1 replacements park + // CR 616.1 mid-entry, and the answer drains through `FinalizeTokenEntry`. + let ability = ResolvedAbility::new( + spec_token_effect(vec!["Creature".to_string()], None, "Counter Saproling"), + Vec::new(), + host, + P0, + ); + let mut events = Vec::new(); + token::resolve(runner.state_mut(), &ability, &mut events).expect("the token resolves"); + let resume_events = answer_counter_order(&mut runner, "the token's enter_with_counters"); + let (site_obj, site_indices) = site_entry(&resume_events, "the FinalizeTokenEntry resume"); + + assert_eq!(runner.state().turn_number, turn_start, "one turn"); + let life_delta = life_of_p0(runner.state()) - life_start; + ( + runner, + SiteRun { + site_obj, + site_indices, + life_delta, + batch_indices, + }, + ) +} + +/// S3a (CR 616.1 + CR 603.6a), unattached arm: a spec token whose entry was postponed across a +/// counter-ordering pause fires the batched ETB trigger when it enters after a routed batch. +/// +/// REVERT-PROBE (discriminating, RUN): restore the private `counters.rs::push_token_entry_events` +/// clone, repoint the `FinalizeTokenEntry` arm back at it, and restore its +/// `record_battlefield_entry` ⇒ life delta 2→1, M2 1→0, M3 [2]→[0], M4 fails. +#[test] +fn counter_paused_token_entry_after_a_token_batch_fires_the_batched_trigger() { + let (runner, run) = drive_s3a_unattached(true); + assert_eq!( + run.life_delta, 2, + "the counter-paused token's entry fires the batched trigger AGAIN" + ); + assert_site_records(&runner, &run, 2); +} + +// ── S3a · counters.rs FinalizeTokenEntry (attach arm) ──────────────────────────────────────── + +fn drive_s3a_attached(mint_batch: bool) -> (GameRunner, SiteRun) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let host = scenario.add_creature(P0, "Batched Watcher", 1, 1).id(); + let attach_host = scenario.add_creature(P0, "Equipped Host", 2, 2).id(); + let painter = scenario + .add_creature_from_oracle(P0, "Painter's Servant", 1, 3, PAINTERS_SERVANT_ORACLE) + .id(); + // The entrant is an ARTIFACT Equipment, which the creature-scoped pair would reject. + stage_any_permanent_counter_pair(&mut scenario); + let mut runner = scenario.build(); + install_host_trigger(&mut runner, host); + // `add_creature_from_oracle` places the object directly on the battlefield without running the + // entry pipeline, so Painter's "As this creature enters, choose a color" never raised its + // `NamedChoice` and `chosen_color()` would answer `None` — leaving the colour instrument below + // inert. Stage the choice the pipeline would have recorded. + runner + .state_mut() + .objects + .get_mut(&painter) + .expect("Painter's Servant is on the battlefield") + .chosen_attributes + .push(ChosenAttribute::Color(ManaColor::Blue)); + + let (life_start, turn_start, batch_indices) = open_turn(&mut runner, host, mint_batch); + + let ability = ResolvedAbility::new( + spec_token_effect( + vec!["Artifact".to_string(), "Equipment".to_string()], + Some(TargetFilter::ParentTarget), + "Bladed Rig", + ), + vec![TargetRef::Object(attach_host)], + host, + P0, + ); + let mut events = Vec::new(); + token::resolve(runner.state_mut(), &ability, &mut events).expect("the token resolves"); + let resume_events = answer_counter_order(&mut runner, "the Equipment token's counters"); + let (site_obj, site_indices) = site_entry(&resume_events, "the attached FinalizeTokenEntry"); + + // Reach-guard for the ATTACH arm specifically: without this the fixture would be a second copy + // of the unattached test. + assert_eq!( + runner.state().objects[&site_obj].attached_to, + Some(AttachTarget::Object(attach_host)), + "the Equipment token must enter attached (CR 301.5) — this is what makes the entry record \ + post-`flush_layers`" + ); + + assert_eq!(runner.state().turn_number, turn_start, "one turn"); + let life_delta = life_of_p0(runner.state()) - life_start; + ( + runner, + SiteRun { + site_obj, + site_indices, + life_delta, + batch_indices, + }, + ) +} + +/// S3a (CR 616.1 + CR 301.5 + CR 603.6a), ATTACH arm. Two claims: +/// +/// (A) FIX — unconditional discriminator, identical to the unattached arm's. +/// REVERT-PROBE (discriminating, RUN): restore `counters.rs::push_token_entry_events`, repoint +/// `FinalizeTokenEntry` back at it, restore its `record_battlefield_entry` ⇒ life delta 2→1, +/// M2 1→0, M3 [2]→[0], M4 fails. +/// +/// (B) ORDERING — the record point moves from before `attach::attach_to` to after it, and +/// `attach_to` ends in a SYNCHRONOUS `flush_layers`, so every layer-derived field of the entry +/// record (`colors`, `keywords`, types, controller) is now snapshotted post-flush. That is the +/// CR-correct point (it is what the unpaused twin in `token.rs` already does: attach, then call +/// the same helper). REVERT-PROBE (conditional, RUN and journal either way): move the S3a record +/// point back to before the attach block. If the `colors` equality below flips, this test covers +/// the ordering change too; if it does not, the ordering change ships untested and that is +/// disclosed rather than papered over. +#[test] +fn counter_paused_attached_token_entry_after_a_token_batch_fires_the_batched_trigger() { + let (runner, run) = drive_s3a_attached(true); + assert_eq!( + run.life_delta, 2, + "the counter-paused ATTACHED token's entry fires the batched trigger AGAIN" + ); + assert_site_records(&runner, &run, 2); + + // FIXTURE-VALIDITY GUARD (fail-loud, deliberately identical on both trees): Painter's Servant + // must actually colour the Equipment token. An uncoloured token makes the ordering assertion + // below vacuous. + let live_color = runner.state().objects[&run.site_obj].color.clone(); + assert!( + !live_color.is_empty(), + "Painter's Servant must colour the Equipment token — an inert fixture makes the ordering \ + assertion vacuous" + ); + + // (B) ORDERING: the entry record is taken after `attach_to`'s synchronous `flush_layers`, so + // its layer-derived colours agree with the live object. + let entry_row = runner + .state() + .battlefield_entries_this_turn + .iter() + .find(|record| record.object_id == run.site_obj) + .expect("the attached token has a CR 608.2i battlefield-entry row"); + assert_eq!( + entry_row.colors, live_color, + "the entry record snapshots the token's post-attach, post-flush colours (CR 608.2i)" + ); +} + +// ── S3b · counters.rs FinalizeCopyTokenEntry ───────────────────────────────────────────────── + +fn drive_s3b(mint_batch: bool) -> (GameRunner, SiteRun) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let host = scenario.add_creature(P0, "Batched Watcher", 1, 1).id(); + let watchdog = scenario + .add_creature(P0, "Faithful Watchdog", 0, 0) + .with_plus_counters(3) + .from_oracle_text_with_keywords(&["Vigilance"], FAITHFUL_WATCHDOG_ORACLE) + .id(); + // The copy is a CREATURE, so the creature pair admits it — and unlike S5 there are NO + // modifications, so stage 1 cannot pause and the `etb_counters` loop is what parks CR 616.1. + stage_creature_counter_pair(&mut scenario); + let mut runner = scenario.build(); + install_host_trigger(&mut runner, host); + + let (life_start, turn_start, batch_indices) = open_turn(&mut runner, host, mint_batch); + + let ability = ResolvedAbility::new( + copy_token_effect(Vec::new()), + vec![TargetRef::Object(watchdog)], + host, + P0, + ); + let mut events = Vec::new(); + token_copy::resolve(runner.state_mut(), &ability, &mut events).expect("the copy resolves"); + let resume_events = answer_counter_order(&mut runner, "the copy's seeded etb_counters"); + let (site_obj, site_indices) = site_entry(&resume_events, "the FinalizeCopyTokenEntry resume"); + + assert_eq!(runner.state().turn_number, turn_start, "one turn"); + let life_delta = life_of_p0(runner.state()) - life_start; + ( + runner, + SiteRun { + site_obj, + site_indices, + life_delta, + batch_indices, + }, + ) +} + +/// S3b (CR 616.1 + CR 707.2 + CR 603.6a): a copy token whose SEEDED etb-counter placement parked +/// the ordering choice fires the batched ETB trigger when it enters after a routed batch. +/// +/// REVERT-PROBE (discriminating, RUN): restore `counters.rs::push_token_entry_events`, repoint the +/// `FinalizeCopyTokenEntry` arm back at it, restore its `record_battlefield_entry` ⇒ life delta +/// 2→1, M2 1→0, M3 [2]→[0], M4 fails. +#[test] +fn counter_paused_copy_token_entry_after_a_token_batch_fires_the_batched_trigger() { + let (runner, run) = drive_s3b(true); + assert_eq!( + run.life_delta, 2, + "the counter-paused copy token's entry fires the batched trigger AGAIN" + ); + assert_site_records(&runner, &run, 2); +} + +// ── S2 · counters.rs InjectPredefinedTokenAbilities (incubate resume) ──────────────────────── + +fn drive_s2(mint_batch: bool) -> (GameRunner, SiteRun) { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + let host = scenario.add_creature(P0, "Batched Watcher", 1, 1).id(); + // The Incubator is a colorless ARTIFACT: the creature-scoped pair provably does NOT admit it, + // so this fixture must use the any-permanent pair or it would never pause. + stage_any_permanent_counter_pair(&mut scenario); + let mut runner = scenario.build(); + install_host_trigger(&mut runner, host); + + let (life_start, turn_start, batch_indices) = open_turn(&mut runner, host, mint_batch); + + let ability = ResolvedAbility::new( + Effect::Incubate { + count: QuantityExpr::Fixed { value: 1 }, + }, + Vec::new(), + host, + P0, + ); + let mut events = Vec::new(); + incubate::resolve(runner.state_mut(), &ability, &mut events).expect("the incubate resolves"); + // The entry is DEFERRED behind the counter pause: nothing has entered yet. + assert!( + !events + .iter() + .any(|event| matches!(event, GameEvent::ZoneChanged { .. })), + "the Incubator's entry is deferred until its counters settle — a ZoneChanged here means \ + the fixture never paused and is measuring the unpaused incubate route instead" + ); + let resume_events = answer_counter_order(&mut runner, "the Incubator's counter"); + let (site_obj, site_indices) = site_entry(&resume_events, "the incubate resume"); + + // POSITIVE CONTROL: both doublers really applied. Base is 1; either alone reaches 2; the pair + // reaches 3 (`(1*2)+1`) or 4 (`(1+1)*2`) depending on the chosen order. A reading of 2 means + // Ozolith's parsed `quantity_modification` is not `Plus{1}` — escalate, do not weaken to `>=2` + // (that would be satisfied by either doubler alone and so is vacuous). + assert!( + runner.state().objects[&site_obj] + .counters + .get(&CounterType::Plus1Plus1) + .copied() + .unwrap_or(0) + >= 3, + "both counter doublers must have applied to the Incubator; got {:?}", + runner.state().objects[&site_obj].counters + ); + + assert_eq!(runner.state().turn_number, turn_start, "one turn"); + let life_delta = life_of_p0(runner.state()) - life_start; + ( + runner, + SiteRun { + site_obj, + site_indices, + life_delta, + batch_indices, + }, + ) +} + +/// S2 (CR 616.1 + CR 603.6a): an Incubator whose entry was deferred behind a counter-ordering +/// pause fires the batched ETB trigger when it enters after a routed token batch. +/// +/// REVERT-PROBE (discriminating, RUN): replace the `zones::record_and_emit_entry_from_no_zone` +/// call in the `InjectPredefinedTokenAbilities` arm with the hand-rolled +/// `snapshot_for_zone_change` + `state.zone_changes_this_turn.push_back(…)` + +/// `events.push(ZoneChanged)`, and restore its `record_battlefield_entry` ⇒ life delta 2→1, +/// M1 fails (a row at position 2 carrying index 0), M3 [2]→[0], M4 fails. +#[test] +fn incubate_resume_entry_after_a_token_batch_fires_the_batched_trigger() { + let (runner, run) = drive_s2(true); + assert_eq!( + run.life_delta, 2, + "the resumed Incubator's entry fires the batched trigger AGAIN" + ); + assert_site_records(&runner, &run, 2); +} + +// ── T8 · single-entry controls, one per site class ─────────────────────────────────────────── +// +// Each drives the SAME `drive_` body with no reach batch, into an EMPTY ledger. The +// instrument is the EMITTED index, which reads `[0]` on BOTH trees (unfixed: the placeholder; +// fixed: `len() == 0`) — a deliberate no-flip. These are NOT discriminators and carry no +// revert-probe: their job is to prove each fixture reaches its site and fires at all, so that the +// `== 2` failure of the tests above on an unfixed tree reads as SUPPRESSION rather than fixture +// breakage. They deliberately do not call `assert_site_records` or `ledger_index` — `ledger_index` +// panics on the unfixed tree at the five sites that record no row, which would destroy the +// control property. + +fn assert_single_entry_control(run: &SiteRun) { + assert_eq!( + run.life_delta, 1, + "the site's entry alone fires the batched trigger exactly once" + ); + assert_eq!( + run.site_indices, + vec![0], + "the first entry of an empty-ledger turn legitimately takes index 0" + ); + assert!( + run.batch_indices.is_empty(), + "the control drives no reach batch" + ); +} + +#[test] +fn t8_conjure_alone_into_an_empty_ledger_fires_once() { + let (_runner, run) = drive_s1(false); + assert_single_entry_control(&run); +} + +#[test] +fn t8_gift_token_alone_into_an_empty_ledger_fires_once() { + let (_runner, run) = drive_s6(false); + assert_single_entry_control(&run); +} + +#[test] +fn t8_copy_token_tail_alone_into_an_empty_ledger_fires_once() { + let (_runner, run) = drive_s4(false); + assert_single_entry_control(&run); +} + +#[test] +fn t8_modification_paused_copy_alone_into_an_empty_ledger_fires_once() { + let (_runner, run) = drive_s5(false); + assert_single_entry_control(&run); +} + +#[test] +fn t8_counter_paused_token_alone_into_an_empty_ledger_fires_once() { + let (_runner, run) = drive_s3a_unattached(false); + assert_single_entry_control(&run); +} + +#[test] +fn t8_counter_paused_copy_alone_into_an_empty_ledger_fires_once() { + let (_runner, run) = drive_s3b(false); + assert_single_entry_control(&run); +} + +#[test] +fn t8_incubate_resume_alone_into_an_empty_ledger_fires_once() { + let (_runner, run) = drive_s2(false); + assert_single_entry_control(&run); +} From b8b8a3cb0bf138cf12db6b1b95a05aa3fa3dcce2 Mon Sep 17 00:00:00 2001 From: lgray Date: Tue, 4 Aug 2026 19:29:00 -0500 Subject: [PATCH 2/3] fix(test): fail closed on a malformed top-level `fn` header in the entry census MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `top_level_fn_headers` read `tokens[at + 1]` with no bounds check, so a column-0 header whose final whitespace token is the bare `fn` aborted the census with an index-out-of-bounds panic instead of its designed `Err` diagnostic — the wrong failure mode for what is really an unhandled header shape. Route the missing name token through the existing line-numbered `Err` path. Reachability measured before fixing: zero lines in `crates/engine/src` currently have this shape, so the panic was latent, not live. That zero carries a positive control — the same detector returns 2 on a synthetic corpus containing the shape. Test `bare_trailing_fn_header_is_an_error_not_a_panic` is non-vacuous and two-sided discriminating, both demonstrated rather than asserted: - NON-VACUOUS: fixtures use the prefixes `""` and `"pub "`, both present in `FN_PREFIX_ALLOW_SET`, so they clear the earlier unrecognised-prefix `Err` arm and actually reach the name-token read. An unknown-prefix fixture would return `Err` upstream and assert nothing about this line. - DROP (guard removed): arm 1 fails with `index out of bounds: the len is 1 but the index is 1`, reproducing the reported defect. - TRIVIALIZE (always `Err`): arm 2 fails on the happy path, so a blanket `Err` cannot satisfy the test. Raised independently in review by the maintainer and by the automated reviewer. Assisted-by: ClaudeCode:claude-opus-5 --- .../battlefield_entry_authority_census.rs | 47 ++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/crates/engine/tests/integration/battlefield_entry_authority_census.rs b/crates/engine/tests/integration/battlefield_entry_authority_census.rs index a939585c7c..702c488aef 100644 --- a/crates/engine/tests/integration/battlefield_entry_authority_census.rs +++ b/crates/engine/tests/integration/battlefield_entry_authority_census.rs @@ -933,7 +933,19 @@ fn top_level_fn_headers(src: &str) -> Result, String> { n + 1 )); } - let name = tokens[at + 1] + // A column-0 header whose LAST token is the bare `fn` puts the name on the next line, + // breaking the one-line-header assumption documented above. Route it through `Err` — a + // raw index here would abort the census with an out-of-bounds panic, i.e. the wrong + // failure mode for what is really an unhandled header shape. + let Some(name_token) = tokens.get(at + 1) else { + return Err(format!( + "line {}: column-0 header ends at the bare `fn` token, so the function name is \ + not on this line. The census resolves a hit's enclosing function by that name, \ + and cannot do so here. Re-check the single-line-header assumption above.", + n + 1 + )); + }; + let name = name_token .split(['(', '<']) .next() .unwrap_or_default() @@ -943,6 +955,39 @@ fn top_level_fn_headers(src: &str) -> Result, String> { Ok(out) } +/// A column-0 header ending at the bare `fn` token is reported, not panicked on. +/// +/// NON-VACUITY: both fixtures carry a prefix that IS in `FN_PREFIX_ALLOW_SET` (`""` and `"pub "`), +/// so they clear the unrecognised-prefix `Err` above and actually reach the name-token read. A +/// fixture with an unknown prefix would return `Err` from the earlier arm and assert nothing here. +/// DISCRIMINATION is two-sided: dropping the `tokens.get(at + 1)` guard makes arm 1 panic with an +/// index-out-of-bounds instead of returning `Err`, and trivialising the function to always return +/// `Err` fails arm 2. No current `crates/engine/src` line has this shape (measured: zero), so this +/// pins the failure MODE of an unhandled header shape rather than a live occurrence. +#[test] +fn bare_trailing_fn_header_is_an_error_not_a_panic() { + // Arm 1 — the shape CodeRabbit flagged: name is not on the header line. + for src in ["fn", "pub fn", "fn\npub fn real_one() {}"] { + let err = top_level_fn_headers(src) + .expect_err("a header ending at the bare `fn` token must be reported as `Err`"); + assert!( + err.contains("bare `fn` token"), + "error must name the unhandled header shape, got: {err}" + ); + } + + // Arm 2 — the happy path still resolves names, so arm 1 cannot be satisfied by a + // blanket `Err`. + let headers = + top_level_fn_headers("pub fn alpha(x: u8) {}\nfn beta() {}\n fn nested() {}") + .expect("well-formed single-line headers must parse"); + assert_eq!( + headers, + vec![(1, "alpha".to_string()), (2, "beta".to_string())], + "indented `fn` is not a column-0 header and must not be collected" + ); +} + /// The enclosing top-level `fn` of `line`: the LATEST collected header at or before it. fn enclosing_fn(headers: &[(usize, String)], line: usize) -> Option<&str> { headers From ee4446fdb649a0d1356f80bf13622f296129ed44 Mon Sep 17 00:00:00 2001 From: lgray Date: Tue, 4 Aug 2026 19:29:20 -0500 Subject: [PATCH 3/3] docs(engine): let the incubate entry-routing comment explain its own CR 608.2i half MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The automated review read `CR 400.7 + CR 608.2i + CR 603.2c` on this header and asked for CR 608.2i to be deleted here and at five sibling sites, on the grounds that it "does not define battlefield-entry bookkeeping". The citation is correct and stays; the paragraph under it was the actual gap. `restrictions::record_zone_change` writes BOTH ledgers in one call: it appends the CR 400.7 zone-change row (whose length is the index allocator) and, when `to_zone == Zone::Battlefield`, calls `record_battlefield_entry` for the CR 608.2i entry row. Every routed site here reaches the battlefield, so both rules describe writes the call actually performs. Unlike its five siblings, this paragraph explained only the index half — its body was entirely zone-change index and batched-trigger dedup and never mentioned the entry ledger, leaving the CR 608.2i half of its own citation unexplained. Per the repo convention that a CR annotation's description is mandatory and grep output must be self-documenting, state the entry-ledger half explicitly instead of dropping a true citation. Comment-only; no behaviour change. Assisted-by: ClaudeCode:claude-opus-5 --- crates/engine/src/game/effects/incubate.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/engine/src/game/effects/incubate.rs b/crates/engine/src/game/effects/incubate.rs index 6cb3bc296c..afc7e33441 100644 --- a/crates/engine/src/game/effects/incubate.rs +++ b/crates/engine/src/game/effects/incubate.rs @@ -115,6 +115,11 @@ pub fn resolve( // `zones::record_and_emit_entry_from_no_zone` — the single `from: None → Battlefield` // authority, which assigns this turn's zone-change index through // `restrictions::record_zone_change` and writes it back onto the record it emits. + // That one call writes BOTH ledgers, which is why both rules are cited here: the CR 400.7 + // zone-change row (whose length IS the index allocator) and — because `to_zone` is + // `Battlefield` — the CR 608.2i battlefield-entry row, via `record_battlefield_entry`. The + // latter is the look-back journal that a permanent which has since left still counts in, so + // re-recording either ledger at this call site would double-count it. // `snapshot_for_zone_change` leaves that index at its `0` placeholder, and the batched // zone-change replay guard (`triggers.rs`) dedups on `(definition_ref, turn_zone_change_index)` // read off the EVENT, so an unrouted record aliases this Incubator onto occurrence `0` and a