Zone-change pipeline + type dedup + parser grammar consolidation

[matthewevans]

Summary

Architectural remediation batch 2: the zone-change pipeline (single entry + proof-token delivery), the type-dedup batch, parser grammar consolidation, and the verbatim-cluster cleanup. 50 commits squashed; contributors should rebase after this lands.

Zone-change pipeline (Phases A–D + W1–W2)

• Single entry: all zone moves route through move_object/execute_zone_move → replace_event consult → ApprovedZoneChange proof token → deliver + delivery tail (CR 614.1c entry counters/tap state, CR 306.5b/310.4b intrinsic loyalty, CR 701.24a library auto-shuffle).
• Centralized park: replacement::park_waiting_for happens inside execute_zone_move — callers can no longer forget to park on NeedsChoice.
• PendingBatchDeliveries (generalized from mill) with mods/attribution + typed BatchCompletion continuations (surveil keep-on-top, manifest-dread cleanup, rest piles).
• Rules fixes found by review, each with fail-first tests: mill batch resumes across per-card replacement choices; non-destroy SBA deaths consult Moved redirects (CR 704.5g + 614.6); plain discard consults Moved redirects; self-ETB Moved replacements scoped to battlefield entry (18 sites stamped destination_zone); paused fizzle runs the resolution epilogue; stack-resident objects' own Moved redirects scanned on stack departure; mulligan shuffle-back no longer fires per-card auto-shuffles; debug zone changes are fully inert.
• Exempt causes (mulligan CR 103.5, elimination CR 800.4a, casting CR 601.2a, debug) skip the consult but keep unconditional guards (CR 111.8 token cessation, B→B no-op).

Type dedup

• Effect::AddCounter folded into Effect::PutCounter (serde alias kept).
• sorcery_speed: bool deleted → ActivationRestriction::AsSorcery + is_sorcery_speed() (CR 602.5d), with deserialize migration for legacy payloads.
• ActivationCadence folded into ActivationRestriction (Crew once_per_turn).

Parser consolidation

• Three parallel duration grammars unified into oracle_nom/duration.rs (single authority).
• parse_restriction_modes rewritten as negation × verb-phrase-list grammar (atoms compose; no permutation enumeration).
• Verbatim sentence clusters in subject.rs/sequence.rs replaced with nom single-authority combinators.
• Single authority for unparsed text: Effect::unimplemented(name, fragment) constructor, hand-built literals gated in CI.

CI

• Parser-combinator gate extended (.rfind/.split/.splitn + long-literal equality); engine-authority and skill-doc gates added.
• Coverage ratchet made informational on push-to-main (kept blocking for PRs/merge queue).

Verification

• Full Tilt gate (clippy, test-engine, test-ai) green on each review round; six review rounds across implementation batches with an isolated opus reviewer.
• cargo check --workspace --all-targets green in the ship worktree post-merge with origin/main.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request refactors the zone-change pipeline to route various moves (such as countered spells, milled cards, mass bounce, and state-based action departures) through the standard replacement pipeline, ensuring that redirects like Rest in Peace are correctly applied. It also unifies sorcery-speed timing representation under ActivationRestriction::AsSorcery, consolidates counter-placement effects into PutCounter, and updates the Crew keyword's once-per-turn restriction. Feedback highlights critical issues where early-returning on battlefield-entry choices (such as Clone copies) in reveal_until.rs and engine_resolution_choices.rs can permanently strand cards in temporary zones. Additionally, the reviewer noted a state corruption risk in zone_pipeline.rs regarding unhandled aura attachment choices and a parsing failure in subject.rs when handling restriction clauses with trailing periods.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

[HIGH] Potential card stranding on battlefield-entry choice.

Why it matters: If the revealed card has an as-enters replacement effect (such as Clone's copy choice, or choosing a color/type/Devour), move_object will return NeedsChoice. Early-returning Ok(()) here bypasses the subsequent move_rest call, permanently stranding the remaining revealed cards in the temporary zone instead of returning them to the library.

Suggested fix: Defer the rest-pile movement/cleanup using a BatchCompletion continuation or similar mechanism, rather than a bare early return.

[matthewevans]

Confirmed and fixed in 89de174. The kept card's battlefield entry genuinely can pause — an Aura among the revealed cards (Polymorph/Proteus-class can hit one) surfaces a CR 303.4f host choice, and competing ETB replacements surface a CR 616.1 prompt — and the early return stranded the misses. The rest-pile move + marker cleanup now defer onto the parked batch tail via a typed BatchCompletion::RevealRestPile, mirroring the surveil/manifest-dread rest-pile pattern, so it runs exactly once after the entry resolves.

[gemini-code-assist]

[HIGH] Potential card stranding on battlefield-entry choice in RevealUntilKeptChoice.

Why it matters: If the accepted card has an as-enters replacement effect (like Clone), move_object will return NeedsChoice. Early-returning here bypasses the move_rest call at the end of the block, permanently stranding the misses (the rest of the revealed cards) in the temporary zone instead of putting them into the decline/rest destination.

Suggested fix: Defer the rest-pile movement/cleanup using a BatchCompletion continuation or similar mechanism, rather than a bare early return.

[matthewevans]

Same root cause as the reveal_until::resolve path — confirmed and fixed identically in 89de174. On an accept-to-battlefield pause we now stash a RevealRestPile completion carrying the misses and their markers; the drain moves them after the kept card's choice resolves instead of stranding them.

[gemini-code-assist]

[HIGH] Potential card stranding on battlefield-entry choice in SelectCards (dig).

Why it matters: If a kept card has an as-enters replacement effect (like Clone), move_object will return NeedsChoice. Early-returning here bypasses the subsequent movement of the unkept cards to the rest destination, permanently stranding them in the temporary zone.

Suggested fix: Defer the rest-pile movement/cleanup using a BatchCompletion continuation or similar mechanism, rather than a bare early return.

[matthewevans]

Confirmed and fixed in 89de174, with a discriminating test (dig_rest_pile_not_stranded_when_kept_aura_pauses_on_attachment_choice). A dig that keeps an Aura onto the battlefield with two legal hosts pauses on the CR 303.4f attachment choice; pre-fix the unkept cards stayed in the library (left: Library, right: Graveyard). The fix defers the rest-pile move plus the dig-specific tracked-set publish and continuation wiring onto the parked batch tail.

[gemini-code-assist]

[MEDIUM] Silent state corruption risk on NeedsAuraAttachmentChoice.

Why it matters: If a batch flow ever triggers NeedsAuraAttachmentChoice, the tail is stashed in pending_batch_deliveries, but because aura-host choices do not route through the replacement-choice resume path, drain_pending_batch_deliveries will not be called. This leaves a stale tail that will be silently and incorrectly drained by the next unrelated replacement-choice resume, causing severe state corruption.

Suggested fix: Instead of silently stashing the tail and returning NeedsChoice, trigger a loud failure (e.g., panic! or unreachable!) or handle the aura-target resolution path explicitly to prevent silent corruption.

[matthewevans]

Real hole, now closed in 89de174. The ReturnAsAuraTarget resume path drained pending_continuation but not pending_batch_deliveries — fine for mill (Battlefield isn't a mill destination) but live for the reveal/dig kept-Aura sites. Both branches of that handler now drain the parked batch tail, which is what makes the rest-pile deferral actually fire on the aura path (verified by the new dig test, which pauses precisely on the CR 303.4f choice).

[gemini-code-assist]

[MEDIUM] Parsing failure on restriction clauses with trailing periods.

Why it matters: all_consuming expects the entire input to be consumed. If lower contains a trailing period (e.g., "can't attack or block."), parse_restriction_list_atom will parse "block", leaving the trailing period unconsumed, which causes all_consuming to fail. Trailing periods are common in Oracle text clauses.

Suggested fix: Trim trailing periods and whitespace from lower before parsing, or allow an optional trailing period in the parser.

pub(crate) fn parse_restriction_modes(lower: &str) -> Option<Vec<StaticMode>> {
    // Negation prefix x verb-phrase-list grammar (CLAUDE.md "Compose nom
    // combinators, don't enumerate permutations"): "can't"/"cannot" applies
    // once and distributes over a comma/or-separated list of
    // parse_restriction_list_atom, covering every compound wording
    // without enumerating the cross-product. Parameterized forms that carry
    // a trailing filter ("... except by <filter>", "... by <filter>") fail the
    // all_consuming list and fall through to their dedicated arms below.
    let cleaned = lower.trim().trim_end_matches('.');
    if let Ok((_, atom_modes)) = all_consuming(preceded(
        (
            alt((tag::<_, _, OracleError<'_>>("can't"), tag("cannot"))),
            tag(" "),
        ),
        separated_list1(
            alt((tag(", or "), tag(", "), tag(" or "))),
            parse_restriction_list_atom,
        ),
    ))
    .parse(cleaned)
    {
        return Some(atom_modes.concat());
    }
    // CR 701.19c: "~ can't be regenerated" - marks the subject so regeneration
    // shields are not applied. Backstop for the "cannot" phrasing and any caller
    // that routes through the generic " can't " / " cannot " split before
    // reaching the dedicated arm in try_parse_subject_restriction_clause.
    // Kept outside the atom list: it tolerates a trailing period.
    if parse_cant_be_regenerated_predicate(cleaned).is_ok() {
        return Some(vec![StaticMode::CantBeRegenerated]);
    }

References

1. Avoid verbatim string equality for parsing Oracle phrases as it bypasses the robust nom-based parser and creates fragile matches. Instead, decompose compound phrases into modular, reusable parsers for constituent parts and compose them using idiomatic combinator aggregates to prevent combinatorial explosion and improve maintainability.

[matthewevans]

Confirmed real and fixed in 234a87f. The terminal period does reach parse_restriction_modes — try_parse_subject_restriction_clause's predicate keeps it when no trailing duration strips it — so "can't attack or block." fell through all_consuming and the restriction was silently dropped. (The pre-rewrite == matching had the same blind spot, so this was latent, not a regression.) Fixed in the combinator with terminated(list, opt(tag("."))) before eof rather than trimming the string, matching the can't be regenerated arm. Two discriminating tests added.